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

Applying cluster-scoped objects with namespace metadata reveals unexpected behavior #40

Merged
merged 1 commit into from
Sep 20, 2019

Conversation

matlec
Copy link
Contributor

@matlec matlec commented Sep 19, 2019

Describe the bug
While deploying the local path provisioner with k3s using the addon controller / static manifests I encountered the following behavior:

  • when the addon controller discovers and applies the manifest for the first time, the deployment succeeds as expected and all objects are successfully created on the api server
  • when the addon controller tries to apply the manifest a second time (e.g. when the addon object is deleted, when the manifest changes, ...), objects that do not have namespace scope (e.g. ClusterRole or ClusterRoleBinding) but reference a namespace in their metadata (e.g. local-path-provisioner-role or local-path-provisioner-bind) are handled incorrectly. Since those objects are persisted without namespace property, compareSets comes to the conclusion that the existing object (with stripped namespace) should be deleted and a new object should be created. process creates objects before it deletes them. This effectively causes non-namespaced objects to be missing.

Proposed fix
The namespace property of objects that are not scoped by namespace can be safely removed before they are being applied. This helps compareSets to identify existing resources and prevent them from being deleted.

…espace metadata and the object has been applied before, apply() will delete the object
@matlec
Copy link
Contributor Author

matlec commented Sep 19, 2019

As a side note, the local path provisioner manifest should probably be fixed and have the namespace removed from cluster-wide objects :-)

@ibuildthecloud
Copy link
Contributor

Cool, thanks!

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.

2 participants