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

Update Kubernetes dependencies to version 0.18.6 to resolve API compatibility problems #17

Merged
merged 1 commit into from
Jul 28, 2020

Conversation

seh
Copy link
Contributor

@seh seh commented Jul 28, 2020

Motivation

Per preceding discussion in the "kops-users" channel of the "Kubernetes" Slack team, k8s-ec2-srcdst is unable to function correctly against a Kubernetes API server running version 1.18.6. It logs messages like the following repeatedly, and is never able to update the source/destination check on any of the ENIs attached to the EC2 instances hosting these Kubernetes nodes.

I0727 20:54:21.655498       1 main.go:42] k8s-ec2-srcdst: v0.2.2
E0727 20:54:21.670824       1 reflector.go:205] github.com/ottoyiu/k8s-ec2-srcdst/cmd/k8s-ec2-srcdst/main.go:48: Failed to list *v1.Node: v1.NodeList: Items: []v1.Node: v1.Node: ObjectMeta: v1.ObjectMeta: readObjectFieldAsBytes: expect : after object field, but found t, error found in #10 byte of ...|s":{"k:{\"type\":\"N|..., bigger context ...|v4Address":{}}},"f:status":{"f:conditions":{"k:{\"type\":\"NetworkUnavailable\"}":{".":{},"f:lastHea|...
E0727 20:54:22.674359       1 reflector.go:205] github.com/ottoyiu/k8s-ec2-srcdst/cmd/k8s-ec2-srcdst/main.go:48: Failed to list *v1.Node: v1.NodeList: Items: []v1.Node: v1.Node: ObjectMeta: v1.ObjectMeta: readObjectFieldAsBytes: expect : after object field, but found 1, error found in #10 byte of ...|":{},"v:\"100.96.7.0|..., bigger context ...|f:spec":{"f:podCIDR":{},"f:podCIDRs":{".":{},"v:\"100.96.7.0/24\"":{}},"f:taints":{}}}},{"manager":"|...
E0727 20:54:23.679405       1 reflector.go:205] github.com/ottoyiu/k8s-ec2-srcdst/cmd/k8s-ec2-srcdst/main.go:48: Failed to list *v1.Node: v1.NodeList: Items: []v1.Node: v1.Node: ObjectMeta: v1.ObjectMeta: readObjectFieldAsBytes: expect : after object field, but found t, error found in #10 byte of ...|s":{"k:{\"type\":\"N|..., bigger context ...|v4Address":{}}},"f:status":{"f:conditions":{"k:{\"type\":\"NetworkUnavailable\"}":{".":{},"f:lastHea|...

That is, the client-go library's Kubernetes client is unable to parse fields within the fairly new "metadata.managedFields" JSON field, representing the ObjectMeta.ManagedFields struct field.

Resolution

Upgrade our Kubernetes dependencies to version "v0.18.6"—the latest stable release—and resolve all complaints from golint, go vet, and staticcheck. This includes unexporting several identifiers that didn't require referencing from other packages.

There are more changes I'd consider making, such as introducing an internal directory to preclude others importing most of this code, and using the controller-runtime library to manage the basic plumbing. That can come later.

Testing

I built a container image with these changes, and tested it in a Kubernetes cluster at version 1.18.6 running in AWS. The program worked as intended, without any complaints visible in the logs.

@ottoyiu
Copy link
Owner

ottoyiu commented Jul 28, 2020

Thank you @seh for the PR! The changes lgtm 👍

We had an internal fork with some of the changes you've done in the PR - like bump up client-go dependency, and use of stable API for Nodes. It also includes a re-write to follow more modern conventions for Kubernetes controller - very simarly to https://github.com/kubernetes/sample-controller.

Unfortunately, the engineer that was leading the charge to get this merged back switched teams, so there's not much traction to make that happen right now.

There are more changes I'd consider making, such as introducing an internal directory to preclude others importing most of this code, and using the controller-runtime library to manage the basic plumbing. That can come later.

With that said, looking forward to future contributions!

@ottoyiu ottoyiu merged commit 39247a1 into ottoyiu:master Jul 28, 2020
@seh
Copy link
Contributor Author

seh commented Jul 28, 2020

Thank you for the fast merge, @ottoyiu. Is it possible to publish a release that the kops project could put into use? The Calico CNI implementation in kops uses k8s-ec2-srcdst today, but we'll need a fresh container image to allow it to work with more recent Kubernetes versions.

@seh seh deleted the upgrade-dependencies branch July 28, 2020 19:18
@ottoyiu
Copy link
Owner

ottoyiu commented Jul 28, 2020

@seh I was fixing the build, as the travis CI file was still using golang1.11 to build. I have tagged a release (v0.3.0) and it should show up on Docker Hub shortly. Will edit this comment as soon as I see it there.

Edit: it should be on docker hub now :)
Cheers!

@seh
Copy link
Contributor Author

seh commented Jul 28, 2020

I saw the release come through. I'll prepare a patch for kops to start using it tomorrow morning.

This afternoon I also started on the port to use the controller-runtime library. I don't expect I'll be able to finish it with the time I have left for this work week, but I'll share it as soon as I'm happy with it. With that I also intend to publish Event objects to Nodes indicating when we've succeeded or failed to disable the source/destination check.

@ottoyiu
Copy link
Owner

ottoyiu commented Jul 29, 2020

I saw the release come through. I'll prepare a patch for kops to start using it tomorrow morning.

Thank you :) The image should be here: https://hub.docker.com/layers/ottoyiu/k8s-ec2-srcdst/v0.3.0/images/sha256-843dc19a607d98e6bc531cfcd87eaa0c75cad38f79006273bd6ee4ffb4f1b6b0?context=explore

This afternoon I also started on the port to use the controller-runtime library. I don't expect I'll be able to finish it with the time I have left for this work week, but I'll share it as soon as I'm happy with it.

💯 appreciate it :)

With that I also intend to publish Event objects to Nodes indicating when we've succeeded or failed to disable the source/destination check.

That'll be extremely useful to debug issues, especially when hitting up against AWS rate limits.

@hakman
Copy link

hakman commented Jul 29, 2020

@ottoyiu Kops is adding ARM64 support in the next major version. Would it be ok to submit a PR to build ARM64 images and multi-arch manifest? Would be something simple, very similar to flannel-io/flannel#1327.

@ottoyiu
Copy link
Owner

ottoyiu commented Jul 29, 2020

@hakman if multi-arch is needed, feel free to create a PR :)

@hakman
Copy link

hakman commented Jul 29, 2020

Amazon has just released the c6g, m6g and r6g instances and seems to be some demand for those.
Thanks @ottoyiu !

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.

None yet

3 participants