-
Notifications
You must be signed in to change notification settings - Fork 1
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
Sync node labels and taints #4
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It can now only handle these in its own namespace. The fix was to add `namespace=k3os-config-operator-system` to the RBAC markers for the CR.
This is necessary in order to get secrets and update the nodes.
annismckenzie
force-pushed
the
sync-node-labels
branch
2 times, most recently
from
January 4, 2021 14:10
39929d1
to
b803fd9
Compare
annismckenzie
force-pushed
the
sync-node-labels
branch
2 times, most recently
from
January 6, 2021 22:04
2e990b4
to
587230f
Compare
This is still very much a work in progress.
I need this over in the picl-k3os-image-generator repository.
annismckenzie
force-pushed
the
sync-node-labels
branch
from
January 6, 2021 22:11
587230f
to
6eb31df
Compare
annismckenzie
commented
Jan 8, 2021
annismckenzie
commented
Jan 8, 2021
annismckenzie
commented
Jan 8, 2021
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.
A couple nits that need to be addressed but overall this looks good.
…ed from For now it’s from the environment via the downward API but this might change and the reconciler should not care when this changes.
This is another approach to handle terminal errors in a reconciler. For the case where an object was deleted (like the node, the secret or the K3OSConfig CR object) we have to return a nil error to the caller so that our reconciler isn’t called anymore. I might flesh this out more in the future with some functional options to configure the response error further. For now, this will do.
…conciler There’s now a couple more smaller methods to make this easier to read, understand and test.
This removes the need for the nasty panic I added earlier.
This cuts down on the amount of parameters these need to lug around.
This way we can change the implementation of that and the rest of the code doesn’t need to care.
This creates metadata-only watches on our secret (filtered by the unique label it has) as well as the node the current instance is running on. It’s pretty light on the network and the memory but boy was this a pain to set up. 🤞 that it works correctly.
We’re using the context everywhere so enabling this should be safe.
This feature has been on by default since v1.14 and this operator was never deployed anywhere.
This includes about the same number of tests as the node label reconciler but the implementation is more complex because of the fact that taints have a lot more intricacies than labels.
This mostly deals with updates to comments and removing dead code.
This handles IsNotFound, IsGone and IsForbidden API server error responses.
annismckenzie
force-pushed
the
sync-node-labels
branch
from
January 8, 2021 10:39
204541c
to
4cf2e0a
Compare
annismckenzie
commented
Jan 8, 2021
1. k3OS cluster that's running nominally | ||
2. A (local) directory of YAML files as describes in https://github.com/sgielen/picl-k3os-image-generator#getting-started: | ||
1. A k3OS cluster that's running [nominally](https://joshdance.medium.com/what-does-nominal-mean-when-spacex-mission-control-says-it-39c2d249da27#:~:text=performing%20or%20achieved%20within%20expected,within%20expected%20and%20acceptable%20limits.). | ||
2. A local clone of https://github.com/sgielen/picl-k3os-image-generator. |
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.
This will probably have to point to my fork to work.
This is done – merging. |
annismckenzie
added
enhancement
New feature or request
go
Pull requests that update Go code
labels
Jan 11, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This closes #5.