Skip to content
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

add recreate option for update-policy directive #189

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kvaps
Copy link
Contributor

@kvaps kvaps commented Dec 21, 2020

This PR adds new option:

directives.qbec.io/update-policy: recreate

Which allow to recreate resources when they change.
This might be really useful for updating jobs and some other immutable objects eg. storage class.

see kubernetes/kubernetes#89657 for more details

solves: #64
partially solves: #149

Now post-install and post-upgrade helm hooks can be achieved by rewriting them with directives.qbec.io/update-policy: recreate annotation)

Another trick can be used if you really need to run job every apply, this can be achieved by adding another annotation with random data, eg.:

  annotations: {
    'directives.qbec.io/update-policy': 'recreate',
    'apply-uuid': importstr "/proc/sys/kernel/random/uuid"
  }

@codecov-io
Copy link

codecov-io commented Dec 21, 2020

Codecov Report

Merging #189 (c4ceba0) into master (3ef9f8e) will decrease coverage by 0.38%.
The diff coverage is 9.37%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #189      +/-   ##
==========================================
- Coverage   73.30%   72.92%   -0.39%     
==========================================
  Files          51       51              
  Lines        4252     4284      +32     
==========================================
+ Hits         3117     3124       +7     
- Misses        961      986      +25     
  Partials      174      174              
Impacted Files Coverage Δ
internal/remote/client.go 0.00% <0.00%> (ø)
internal/commands/apply.go 79.33% <100.00%> (+0.27%) ⬆️
internal/commands/directives.go 100.00% <100.00%> (ø)
internal/eval/eval.go 94.61% <0.00%> (ø)
internal/commands/diff.go 85.30% <0.00%> (ø)
internal/model/app.go 94.71% <0.00%> (+0.01%) ⬆️
internal/commands/config.go 89.24% <0.00%> (+0.05%) ⬆️
internal/model/k8s.go 90.00% <0.00%> (+0.41%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3ef9f8e...c4ceba0. Read the comment docs.

return nil, err
}

watcher, err := ri.Watch(metav1.ListOptions{})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the watch algo is correct. You are watching on all objects of that type in the namespace and stopping the watch without checking on the name. If a different object gets deleted, it can trigger the end of your watch

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, solved by fieldSelector:

watcher, err := ri.Watch(metav1.ListOptions{
TimeoutSeconds: &waitTime,
FieldSelector: "metadata.name=" + obj.GetName(),
})

crazy bug

@Andor
Copy link

Andor commented Oct 5, 2021

@gotwarlost any updates here?
The PR is open for a quite some time and contains meaningful changes.
Could you please consider reviewing it again?

@gotwarlost
Copy link
Contributor

will do shortly.

TimeoutSeconds: &waitTime,
FieldSelector: "metadata.name=" + obj.GetName(),
})
if err != nil {
Copy link
Contributor

@gotwarlost gotwarlost Oct 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this correct? The delete logic says if err != nil && !apiErrors.IsNotFound(err) which means a non-existent object will pass that gate as it should.

What will happen if you start watching that non-existent object? What does the watcher return?

Also I think the watcher should be used as an optimization. That is, the invariant we are looking for is that "this object doesn't exist before I try to create it". We should make sure that this invariant is true before calling create

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants