Skip to content

Avoid overwriting default IPFamily of services - #3084

Merged
charith-elastic merged 1 commit into
elastic:masterfrom
charith-elastic:fix-dual-stack-svc
May 15, 2020
Merged

Avoid overwriting default IPFamily of services#3084
charith-elastic merged 1 commit into
elastic:masterfrom
charith-elastic:fix-dual-stack-svc

Conversation

@charith-elastic

Copy link
Copy Markdown
Contributor

In dual-stack clusters, the IPFamily of the service is not empty. Our reconciliation function compares the Spec of the expected against reconciled and considers this to be a difference that requires an update. However, as the ipFamily is an immutable field, the update operation will always fail with the following error message:

{"service.version": "1.2.0-12a254e1", "controller": "elasticsearch-controller", "request": "default/hulk", "error": "Service \"hulk-es-transport\" is invalid: [spec.ipFamily: Invalid value: \"null\": field is immutable, spec.ipFamily: Required value]", "errorCauses": [{"error": "Service \"hulk-es-transport\" is invalid: [spec.ipFamily: Invalid value: \"null\": field is immutable, spec.ipFamily: Required value]"}]}

This PR does the following:

  • If the ipFamily is not defined in the service spec, it will be defaulted to the ipFamily of the reconciled service. This prevents spurious updates that result in the above error.
  • If the ipFamily is explicitly specified by the user in the service spec and it differs from the current value, it will trigger a re-create operation to delete and create the service again.

Testing this is difficult as currently there are no cloud providers with support for dual-stack clusters. Kind is yet to support it as well. However, as the behaviour is triggered by the mere presence of the dual-stack feature gate, it is possible to configure Minikube to trigger the bug as follows:

minikube start --vm-driver=kvm2 --kubernetes-version=1.17.0 --extra-config=apiserver.feature-gates="IPv6DualStack=true" --extra-config=controller-manager.feature-gates="IPv6DualStack=true" --extra-config=kubelet.feature-gates="IPv6DualStack=true" --feature-gates="IPv6DualStack=true" -p k8s117

Fixes #3074

@charith-elastic charith-elastic added >bug Something isn't working v1.2.0 labels May 15, 2020

@sebgl sebgl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Just to make sure I get this right: with this PR we ensure that a K8s cluster with IPv6DualStack enabled does not lead to errors in our service reconciliation.
Which does not mean we correctly support ipv6 or dual stack, as this is something we did not test (yet). Am I right?

@charith-elastic

Copy link
Copy Markdown
Contributor Author

Yes, this just fixes the reconciliation bug reported in #3074. There's no guarantee that IPv6 would work OOTB with ECK as it is hard to test that easily with existing tools.

@charith-elastic
charith-elastic merged commit bd5936e into elastic:master May 15, 2020
@charith-elastic
charith-elastic deleted the fix-dual-stack-svc branch May 15, 2020 13:48
charith-elastic added a commit to charith-elastic/cloud-on-k8s that referenced this pull request May 18, 2020
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.1.2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

spec.ipFamily has to be set in a dual-stack k8s cluster

2 participants