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

Retain existing OwnerReferences #130

Merged
merged 6 commits into from
Feb 8, 2019
Merged

Conversation

wonderhoss
Copy link

This PR ensures that existing OwnerReferences on Secrets managed by the controller are retained when a Secret is reconciled.
When an existing Secret is found, the controller will now merge existing OwnerReferences with its own, ensuring that other controllers can use OwnerReferences safely to track these Secrets.

Fixes #127

@wonderhoss
Copy link
Author

wonderhoss commented Nov 23, 2018

Looking into these failing integration tests.

@wonderhoss
Copy link
Author

Missing RBAC "get" for secret. Fixed now.

@wonderhoss
Copy link
Author

FWIW, we're now running this latest build in production and it has resolved the problem for us.

@wonderhoss
Copy link
Author

@anguslees, any chance to get this merged soon?

cmd/controller/controller.go Show resolved Hide resolved
if err != nil {
return nil, fmt.Errorf("failed to read existing secret: %s", err)
}
existingSecret.Data = newSecret.Data
Copy link
Contributor

Choose a reason for hiding this comment

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

existingSecret may have come from a shared cache above, so you need to copy it before modifying it:

existingSecret = existingSecret.DeepCopy()

Copy link
Author

Choose a reason for hiding this comment

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

Good catch. Fixed.

@anguslees
Copy link
Contributor

otherwise lgtm, thanks!

(apologies for not responding earlier - kubecon, holidays, and other excuses)

@wonderhoss
Copy link
Author

@anguslees No worries. Thanks for taking a look. I think I have addressed your concerns. Could you re-review?

@wonderhoss
Copy link
Author

@anguslees Could you give this another once-over? Keen to getting it merged.

Copy link
Contributor

@anguslees anguslees left a comment

Choose a reason for hiding this comment

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

lgtm. Thanks!

bors r+

bors bot added a commit that referenced this pull request Feb 8, 2019
130: Retain existing OwnerReferences r=anguslees a=gargath

This PR ensures that existing `OwnerReferences` on Secrets managed by the controller are retained when a Secret is reconciled.
When an existing Secret is found, the controller will now merge existing OwnerReferences with its own, ensuring that other controllers can use OwnerReferences safely to track these Secrets.

Fixes #127 

Co-authored-by: Phil Taprogge <philt@pusher.com>
Co-authored-by: Joel Speed <joel.speed@hotmail.co.uk>
@bors
Copy link
Contributor

bors bot commented Feb 8, 2019

Build succeeded

@bors bors bot merged commit 7a9d19a into bitnami-labs:master Feb 8, 2019
@mkmik mkmik modified the milestone: v0.8.0 Jul 19, 2019
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.

Controller drops foreign OwnerReferences
4 participants