🐛 Fix SetControllerReference to consider multiple CRD versions #707
Conversation
An error was returned by a call to `SetControllerReference` if the owner is already set, but with a different apiVersion (eg. `group/v1beta1` instead of `group/v1`). This commit fixes it by ignoring the version in the ref comparison. It still considers the group though. Having an owner with a different version does not return an error, but the call to `SetControllerReference` effectively changes the owner to match the newer apiVersion.
k8s-ci-robot
added
the
cncf-cla: yes
|
Hi @sebgl. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
k8s-ci-robot
added
needs-ok-to-test
DirectXMan12
changed the title
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: DirectXMan12, sebgl The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
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
* Fix controllerutil.SetControllerReference to ignore apiVersions 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 * Set volumeClaimTemplate namespace to match ES namespace 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.
🏃 use k8s 1.14.1 in test scripts
Fixes #699.
An error was returned by the call to
SetControllerReferenceif the owneris already set, but with a different apiVersion (eg.
group/v1beta1instead of
group/v1).This commit fixes it by ignoring the version in the ref comparison. It
still considers the group though. Having an owner with a different
version does not return an error, but the call to
SetControllerReferenceeffectively changes the owner to match thenewer apiVersion.