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

Upgrade AWS VPC CNI to 1.7.0 #9786

Merged
merged 1 commit into from
Aug 20, 2020

Conversation

moshevayner
Copy link
Member

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 20, 2020
@rifelpet
Copy link
Member

I'm wondering what this means exactly in their release notes:

... Because of these changes, upgrades should not be done by just editing the image tag.

Does that mean we should change the updateStrategy to OnDelete? their example manifest has rollingUpdate so we might want to get clarification on that.

@moshevayner
Copy link
Member Author

moshevayner commented Aug 20, 2020

I'm wondering what this means exactly in their release notes:

... Because of these changes, upgrades should not be done by just editing the image tag.

Does that mean we should change the updateStrategy to OnDelete? their example manifest has rollingUpdate so we might want to get clarification on that.

Actually, the current manifest already includes it.
I basically pulled the current manifest from https://raw.githubusercontent.com/aws/amazon-vpc-cni-k8s/release-1.7/config/v1.7/aws-k8s-cni.yaml and did a side-by-side comparison with the changes. This is why there are more changes than just the usual image tag update.

** UPDATE **
Re-reading your comment, I think what they meant is that we should also make sure our manifest is aligned with their example that you mentioned, because it does include a bunch of other changes, mostly to the ENVs and Volumes.
I guess that's what they meant by "don't only update the image tag"?

@rifelpet
Copy link
Member

ahh, my interpretation was "there are changes to the startup sequence which means you shouldnt upgrade the cni pod on an existing node"

I think your interpretation makes more sense. There are additional changes to the manifest beyond just the image tag bump.

Regardless, our prow jobs don't test upgrade scenarios so we may want to test upgrading an existing cluster from 1.6.X to 1.7.0 just to confirm.

@moshevayner
Copy link
Member Author

Good point.
I'll create a test cluster with this version and make sure it works as expected.
Once confirmed it's good, I'll post an update here. Until then -
/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 20, 2020
@moshevayner
Copy link
Member Author

OK, I've just rebuilt the release branch locally with the changes from this PR and built a test cluster.
Everything seems to be operating as expected:

Pods output:

$ kubectl get pods -n kube-system -l k8s-app=aws-node
NAME             READY   STATUS    RESTARTS   AGE
aws-node-2lk6j   1/1     Running   0          109s
aws-node-5d28c   1/1     Running   0          5m11s
aws-node-sltgb   1/1     Running   0          9m7s

Logs output:

$ stern -n kube-system aws-node
+ aws-node-sltgb › aws-node
+ aws-node-2lk6j › aws-node
+ aws-node-5d28c › aws-node
aws-node-sltgb aws-node {"level":"info","ts":"2020-08-20T03:09:55.581Z","caller":"entrypoint.sh","msg":"Install CNI binary.."}
aws-node-sltgb aws-node {"level":"info","ts":"2020-08-20T03:09:55.601Z","caller":"entrypoint.sh","msg":"Starting IPAM daemon in the background ... "}
aws-node-sltgb aws-node {"level":"info","ts":"2020-08-20T03:09:55.615Z","caller":"entrypoint.sh","msg":"Checking for IPAM connectivity ... "}
aws-node-sltgb aws-node {"level":"info","ts":"2020-08-20T03:09:57.652Z","caller":"entrypoint.sh","msg":"Copying config file ... "}
aws-node-sltgb aws-node {"level":"info","ts":"2020-08-20T03:09:57.655Z","caller":"entrypoint.sh","msg":"Successfully copied CNI plugin binary and config file."}
aws-node-sltgb aws-node {"level":"info","ts":"2020-08-20T03:09:57.656Z","caller":"entrypoint.sh","msg":"Foregrounding IPAM daemon ..."}
aws-node-2lk6j aws-node {"level":"info","ts":"2020-08-20T03:17:14.216Z","caller":"entrypoint.sh","msg":"Install CNI binary.."}
aws-node-2lk6j aws-node {"level":"info","ts":"2020-08-20T03:17:14.234Z","caller":"entrypoint.sh","msg":"Starting IPAM daemon in the background ... "}
aws-node-2lk6j aws-node {"level":"info","ts":"2020-08-20T03:17:14.249Z","caller":"entrypoint.sh","msg":"Checking for IPAM connectivity ... "}
aws-node-2lk6j aws-node {"level":"info","ts":"2020-08-20T03:17:16.267Z","caller":"entrypoint.sh","msg":"Copying config file ... "}
aws-node-2lk6j aws-node {"level":"info","ts":"2020-08-20T03:17:16.270Z","caller":"entrypoint.sh","msg":"Successfully copied CNI plugin binary and config file."}
aws-node-2lk6j aws-node {"level":"info","ts":"2020-08-20T03:17:16.271Z","caller":"entrypoint.sh","msg":"Foregrounding IPAM daemon ..."}
aws-node-5d28c aws-node {"level":"info","ts":"2020-08-20T03:13:51.861Z","caller":"entrypoint.sh","msg":"Install CNI binary.."}
aws-node-5d28c aws-node {"level":"info","ts":"2020-08-20T03:13:51.879Z","caller":"entrypoint.sh","msg":"Starting IPAM daemon in the background ... "}
aws-node-5d28c aws-node {"level":"info","ts":"2020-08-20T03:13:51.892Z","caller":"entrypoint.sh","msg":"Checking for IPAM connectivity ... "}
aws-node-5d28c aws-node {"level":"info","ts":"2020-08-20T03:13:53.909Z","caller":"entrypoint.sh","msg":"Copying config file ... "}
aws-node-5d28c aws-node {"level":"info","ts":"2020-08-20T03:13:53.911Z","caller":"entrypoint.sh","msg":"Successfully copied CNI plugin binary and config file."}
aws-node-5d28c aws-node {"level":"info","ts":"2020-08-20T03:13:53.912Z","caller":"entrypoint.sh","msg":"Foregrounding IPAM daemon ..."}

Validating the version:

$ kubectl describe ds aws-node -n kube-system | grep Image | cut -d ":" -f 2-3
        602401143452.dkr.ecr.us-west-2.amazonaws.com/amazon-k8s-cni-init:v1.7.0
      602401143452.dkr.ecr.us-west-2.amazonaws.com/amazon-k8s-cni:v1.7.0

So looks like we're good to go.
/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 20, 2020
@rifelpet
Copy link
Member

👍 thanks for checking that

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 20, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: MoShitrit, rifelpet

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 20, 2020
@k8s-ci-robot k8s-ci-robot merged commit 90fc25a into kubernetes:master Aug 20, 2020
@k8s-ci-robot k8s-ci-robot added this to the v1.19 milestone Aug 20, 2020
@moshevayner moshevayner deleted the vpc-cni-1.7.0 branch August 20, 2020 03:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/addons cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants