-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Add flag to disable the checksum workaround for Flannel VXLAN #9614
Add flag to disable the checksum workaround for Flannel VXLAN #9614
Conversation
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hakman 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 |
@jhohertz We discussed internally if to disable or not the workaround so late in the release cycle for Kops 1.18. It would be acceptable as long a you could test the build from this PR. If that's acceptable for you, I can provide the steps needed to use this build. |
Would be happy to! Not run a pre-merged build before, will scan the docs and let you know if I need some pointers. |
@jhohertz It is very easy: $ wget https://storage.googleapis.com/kops-ci/pulls/pull-kops-e2e-kubernetes-aws-1-18/pull-42cf9dc24b/linux/amd64/kops
$ export KOPS_BASE_URL=https://storage.googleapis.com/kops-ci/pulls/pull-kops-e2e-kubernetes-aws-1-18/pull-42cf9dc24b
$ ./kops create ... |
Thanks, working on it now, just need to tweak my tooling a bit, should have something later this evening. |
So, launching a 1.18.6 cluster with that build, the issues what would indicate a checksum issue do NOT present. 🎉 Are there any other scenarios you'd like me to try? Was wondering about a .5 -> .6 transition, but I'd need to find a long running connectivity test to run... not 100% sure why it would be an issue as in either case (w/ workaround or w/ patch), the checksum should be good on the vxlan packets sent around. I'd like to help clear this up as it might be useful to 1.17 (1.16?) if we can deem it to be safe. |
As I recall, one of the concerns @justinsb raised at Office Hours was that kops checking against a patch level of Kubernetes would be unprecedented. I cannot represent that position, though. I am neutral on this point as I don't think this particular case requires adding to our test matrix. My concerns are around the proximity to the 1.18.0 release and the risk/benefit tradeoff. The risk seems to have been managed as well as it can be, but, as was mentioned during Office Hours, the benefit seems negligible. |
Hmm, I didn't recall this part when I proposed the change. Thanks for the reminder. Considering that it is confirmed that Kubernetes 1.18.6 doesn't have the issue and was further tested by @jhohertz, seems pretty low risk to skip the workaround for 1.18.6+, but leave the final decision to @justinsb . /assign @justinsb |
8e95f63
to
a481283
Compare
Reworked as flag per discussion during office hours. |
/hold cancel |
I think we might need to add this flag (as a no-op) to master branch to prevent upgrade problems. |
@johngmyers Aren't unknown flags just ignored by Kops (which is actually a problem that pops up from time to time) ? |
|
@johngmyers Done: #9782. |
/lgtm |
Last attempt of making newer k8s versions 1.18 play nice with Flannel VXLAN.
Ref: #9543 (comment)
/cc @johngmyers @rifelpet