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

Add aws-k8s-1.19 variant with Kubernetes 1.19 #1256

Merged
merged 2 commits into from
Jan 7, 2021

Conversation

etungsten
Copy link
Contributor

@etungsten etungsten commented Dec 31, 2020

Issue number:
#1239

Description of changes:

For reference, the addition of 1.18 was in #1150.

There are some small changes to the rpm spec file to get cross compilation working. There was an issue with go code generation tools not being compiled for the target architecture.

I removed --volume-plugin-dir since it has since been deprecated and replaced it with the corresponding field in kubelet's config.

Testing done:

  • Conformance testing with 1.19 control plane, all tests passed (tested arm64 variant with conformance test image version of 1.17.9. Currently unable to test with 1.19.6 conformance test image due to Conformance image v1.18.0 fails on arm64 kubernetes/kubernetes#89554, it still exists for 1.19.6)

  • Pods running OK

  • Checked kubelet logs and system logs in general, compared it with the 1.18 variant kubelet logs and found no abnormalities.

  • Confirmed EBS CSI still works.

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

@jhaynes jhaynes linked an issue Jan 4, 2021 that may be closed by this pull request
@etungsten etungsten force-pushed the k8s-1.19 branch 2 times, most recently from 0a21e0d to 7694d94 Compare January 5, 2021 20:31
@etungsten etungsten marked this pull request as ready for review January 5, 2021 21:15
@etungsten
Copy link
Contributor Author

Going to run conformance testing for aarch64 nodes with a v1.17 conformance test image since that one still works on arm64. I'll update the PR description with the results when it's done.

Copy link
Contributor

@tjkirch tjkirch left a comment

Choose a reason for hiding this comment

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

There are some small changes to the rpm spec file to get cross compilation working. There was an issue with go code generation tools not being compiled for the target architecture.

Can you please explain this a bit more? I don't understand what was wrong or why we had to remove those lines, or how it's different than 1.18.

Conformance testing with 1.19 control plane, all tests passed (for x86 only, aarch64 conformance testing is not possible right now due to kubernetes/kubernetes#89554, it still exists for 1.19.4)

Can you do aarch64 conformance testing against a 1.17 cluster? That worked for 1.18.

Also, would you please test CSI? That's been changing for a few k8s releases and would be good to confirm. https://docs.aws.amazon.com/eks/latest/userguide/ebs-csi.html

Would you please also confirm that the system journal doesn't show any new issues compared to 1.18?

variants/aws-k8s-1.19/Cargo.toml Outdated Show resolved Hide resolved
@etungsten
Copy link
Contributor Author

etungsten commented Jan 6, 2021

Can you please explain this a bit more? I don't understand what was wrong or why we had to remove those lines, or how it's different than 1.18.

Before 1.19, one of the golang code generation tool, namely go-bindata, wasn't actually being installed as part of kubernete's build process due to a bug. That bug got fixed in 1.19 by kubernetes/kubernetes@fe0f388. Which meant now we're trying to cross compile the tool which causes the build to fail when it gets to the gen_bindata makefile target.

As for how removing %cross_go_setup and %cross_go_configure fixes the issue, I am not entirely sure. @bcressey can probably answer that better.

Can you do aarch64 conformance testing against a 1.17 cluster? That worked for 1.18.

Yeah, I'll update the PR description with conformance testing with a v1.17 conformance test image which does not have the arm64 issue.

Also, would you please test CSI? That's been changing for a few k8s releases and would be good to confirm. https://docs.aws.amazon.com/eks/latest/userguide/ebs-csi.html

I went through all the steps in that article to set up EBS CSI and verified that a pod can write to an EBS volume.

Would you please also confirm that the system journal doesn't show any new issues compared to 1.18?

I've checked kubelet logs and the journal in general and found nothing out of the normal compared to 1.18

'--volume-plugin-dir' has been deprecated. Now specifying
volumePluginDir via kubelet's config file.
@etungsten
Copy link
Contributor Author

etungsten commented Jan 6, 2021

Can you do aarch64 conformance testing against a 1.17 cluster? That worked for 1.18.

I finished conformance tests using conformance test image version 1.17.9 against a 1.19 control plane. There were several test failures that had to do with API calls timing out due to deprecated Kubernetes API endpoints. But everything else passed.

I tried doing conformance test against a 1.17 control plane but the coredns version that's compatible with 1.19 nodes is not compatible with a 1.17 control plane it seems. (coredns pods unschedulable) In general, newer kubelet versions are not compatible with older control plane versions.

@etungsten
Copy link
Contributor Author

etungsten commented Jan 6, 2021

Push above rebases onto develop to pull in the new SDK, addresses #1256 (comment), updates the packaged k8s 1.19 version from 1.19.4 to 1.19.6.

I'll also move the PR back into draft move as I updated the kubernetes 1.19 version from 1.19.4 to 1.19.6. In the process of rerunning conformance tests.

@etungsten etungsten marked this pull request as draft January 6, 2021 23:21
@etungsten etungsten marked this pull request as ready for review January 7, 2021 04:42
@gregdek gregdek added the status/needs-triage Pending triage or re-evaluation label Jan 7, 2021
@tjkirch tjkirch removed the status/needs-triage Pending triage or re-evaluation label Jan 7, 2021
Comment on lines +54 to +57
%build
export KUBE_BUILD_PLATFORMS="linux/%{_cross_go_arch}"
export GOLDFLAGS="-buildmode=pie -linkmode=external"
make WHAT="cmd/kubelet"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like for these changes to %build and %prep to get backported to the other specs, but that can happen in a different PR.

@bcressey
Copy link
Contributor

bcressey commented Jan 7, 2021

As for how removing %cross_go_setup and %cross_go_configure fixes the issue, I am not entirely sure. @bcressey can probably answer that better.

%cross_go_configure sets GOARCH in the environment, which caused the "host" binaries to be cross-compiled, so go install would refuse to install them into GOPATH. The upstream fix made it so that go would actually try the install rather than failing an earlier check.

The Kubernetes build sets most of the relevant variables so %cross_go_setup has always been somewhat redundant. The main difference is that we're not setting other variables like CGO_CFLAGS, CGO_LDFLAGS, PKG_CONFIG_PATH. But today the build doesn't compile any C files or use pkg-config, so this doesn't really matter.

(%cross_go_setup sets up some directories and symlinks that %cross_go_configure expects, and isn't needed if the other macro isn't used.)

@etungsten etungsten merged commit 1dbacd3 into bottlerocket-os:develop Jan 7, 2021
@etungsten etungsten deleted the k8s-1.19 branch January 7, 2021 22:01
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.

Kubernetes 1.19 support
5 participants