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

Use a bugfixed version of controllerutil.SetControllerReference #2211

Merged
merged 3 commits into from
Dec 4, 2019

Conversation

sebgl
Copy link
Contributor

@sebgl sebgl commented Dec 4, 2019

Fixes #2191.

Goal

Temporarily duplicate the fixed version of
controllerutil.SetControllerReference in our own code, until we get a
proper release of controller-runtime with the fix.

At which point we can safely remove this file and rely on
controllerutil.SetControllerReference again.

controller-runtime PR including the fix:
kubernetes-sigs/controller-runtime#707

Tweaks

The latest version of controllerutil.SetControllerReference does a
safety check on owner vs. object namespace mismatches.
We technically don't need to include a namespace in the
VolumeClaimtemplates of a StatefulSet, but since we know in which
namespace the PVC will be created anyway, it's safe to include the
namespace so we pass the function safety check.

sebgl added 2 commits December 4, 2019 11:19
Temporarily duplicate the fixed version of
`controllerutil.SetControllerReference` in our own code, until we get a
proper release of controller-runtime with the fix.

At which point we can safely remove this file and rely on
`controllerutil.SetControllerReference` again.

controller-runtime PR including the fix:
kubernetes-sigs/controller-runtime#707
The latest version of `controllerutil.SetControllerReference` does a
safety check on owner vs. object namespace mismatches.
We technically don't need to include a namespace in the
VolumeClaimtemplates of a StatefulSet, but since we know in which
namespace the PVC will be created anyway, it's safe to include the
namespace so we pass the function safety check.
@sebgl
Copy link
Contributor Author

sebgl commented Dec 4, 2019

Follow-up issue so we don't forget to remove the temporary code duplication: #2212

@sebgl sebgl mentioned this pull request Dec 4, 2019
Copy link
Contributor

@barkbay barkbay left a comment

Choose a reason for hiding this comment

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

lgtm

@sebgl sebgl merged commit b82e6ff into elastic:master Dec 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug Something isn't working v1.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Existing v1beta1 owner references prevent upgrades to v1
2 participants