-
Notifications
You must be signed in to change notification settings - Fork 887
feat(coderd/database): support exact tags match in AcquireProvisionerJob query #12244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
9b77b42
to
3796e17
Compare
…ecifying exact tag match behaviour
3796e17
to
6c18525
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a nice and concise PR! Left 2 nit-picks.
AND nested.tags <@ @tags :: jsonb | ||
-- Ensure the caller satisfies all job tags if requested, | ||
AND CASE | ||
WHEN @exact_tag_match :: boolean THEN nested.tags = @tags :: jsonb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are both guaranteed to be sorted arrays? Might be a good idea to allow unsorted equality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think they're map[string]string
so we want to completely ignore order in comparisons.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right, then a jsonb
to jsonb
comparison should be fine 👍🏻
Putting the scope of the change here has the unfortunate side-effect of enforcing a global tag policy. I'm going to make additional modifications to modify the scope of acquirer.AcquireJob and have the parameter be passed in from provisionerdserver. This should allow individual behaviour per provisioner daemon as we start a provisionerdserver for each provisioner daemon. |
Alternative solution: #12269 I'm leaning in favour of this approach as it doesn't require another configuration knob. |
Part of #6442
Updates AcquireProvisionerJob query with parameter ExactTagMatch which toggles between existing behaviour (subset matches) or new behaviour (all match).
Adds unit tests for new behaviour.
A separate PR will wire this up to a configuration knob in coderd and provisionerd.