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

INSTALL, conformance-test scripts: apply configuration for aws-k8s-cni v1.6.0 #739

Merged
merged 2 commits into from
Feb 17, 2020

Conversation

etungsten
Copy link
Contributor

Issue #, if available: N/A

Description of changes:
Add install instructions to apply configuration for v1.6.0 of the aws-k8s-cni plugin
Updated sonobuoy conformance test script to apply v1.6.0 of the aws-k8s-cni plugin to the test cluster

Testing done:
Applied configuration for aws-k8s-cni v1.6.0 on a fresh cluster and ran sonobuoy conformance tests over it, tests pass:

✦ $ sonobuoy results 202002140125_sonobuoy_2ed81549-a7da-442a-99f2-5b6f1973a3d3.tar.gz
Plugin: e2e                       
Status: passed                                                                       
Total: 3585                                                                                   
Passed: 204                                                                                                                                    
Failed: 0                                                                             
Skipped: 3381

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Updates install instructions to apply config for version v1.6.0 of the
amazon-k8s-cni plugin
Update setup-test-cluster script to apply configuration for v1.6.0 of
the aws-k8s-cni plugin
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.

It does make sense to me to apply the full config relevant to the version instead of just updating the URI, because they've changed a couple other settings that could be important. I'm most worried about the addition of the health probe; it could be a good thing, but we haven't run with it before. @samuelkarp - do you have any experience with this?

@samuelkarp
Copy link
Contributor

@tjkirch What worries you specifically about the probes?

In general, probes exist to provide information about the state of the application running inside a container; a readiness probe is used to determine whether the application has started and is ready to perform work while a liveness probe is used to determine whether a container should be restarted.

You can find more about probes here and how they interact with the pod lifecycle here.

In this case, I don't see a restartPolicy specified, so I don't know that the liveness probe is going to have any effect. However, I would bias toward assuming that the settings defined there are fine, in that they're specified by the EKS team for use with EKS and other Kubernetes clusters running on AWS.

@tjkirch
Copy link
Contributor

tjkirch commented Feb 17, 2020

@tjkirch What worries you specifically about the probes?

@samuelkarp Nothing worries me specifically, I just had no experience with them in production. I know what they're for, but don't know if they're likely to cause us problems. I'll assume you think not, given your comments and approval.

@etungsten etungsten merged commit 06d0104 into develop Feb 17, 2020
@etungsten etungsten deleted the new-aws-vpc-cni branch February 17, 2020 21:09
@samuelkarp
Copy link
Contributor

I'll assume you think not, given your comments and approval.

Discussed offline yesterday, but I failed to follow up. Yes, I think this is fine.

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.

4 participants