-
Notifications
You must be signed in to change notification settings - Fork 742
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
Calico integration Tests #1906
Calico integration Tests #1906
Conversation
scripts/run-integration-tests.sh
Outdated
ginkgo -v e2e/calico -- --cluster-kubeconfig=$KUBECONFIG --cluster-name=$CLUSTER_NAME --aws-region=$AWS_DEFAULT_REGION --aws-vpc-id=$VPC_ID --calico-version=$calico_version | ||
popd | ||
|
||
if [[ "$DEPROVISION" == false ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think DEPROVISION is set to false only when we want to login to the cluster nodes and debug. As such, automated termination of nodes may not be needed here. Thoughts ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we planning to recycle the cluster? @jayanthvn
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel we should keep this in case the Calico tests interfere with any tests without awareness in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to reuse the cluster since this is run only weekly.
@@ -24,6 +24,9 @@ import ( | |||
type InstallationManager interface { | |||
InstallCNIMetricsHelper(image string, tag string) error | |||
UnInstallCNIMetricsHelper() error | |||
InstallTigeraOperator(version string) error | |||
UninstallTigeraOperator() error | |||
//AddAndUpdateRepository(entry repo.Entry) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove this commented code ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for catching this. Will remove it.
test/e2e/calico/calico_suite_test.go
Outdated
Expect(err).ToNot(HaveOccurred()) | ||
|
||
By("Patching ARM64 node unschedulable") | ||
err = updateNodesSchedulability("beta.kubernetes.io/arch", "arm64", true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we instead use "kubernetes.io/arch" ? I know we do tag with beta still but better to use "kubernetes.io/arch".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make sense. I will change this to not use beta
.
err := f.InstallationManager.InstallTigeraOperator(tigeraVersion) | ||
Expect(err).ToNot(HaveOccurred()) | ||
|
||
By("Patching ARM64 node unschedulable") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason we mark it unschedulable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh is it because star policy is still not supported for ARM instance types? Maybe we can check this - projectcalico/calico#3717 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cordon these arm nodes to avoid Calico tests pods deployed to them. For now the provided tests containers are for AMD.
test/e2e/calico/calico_suite_test.go
Outdated
Container(uiContainer). | ||
Replicas(1). | ||
PodLabel("role", "management-ui"). | ||
NodeSelector("beta.kubernetes.io/arch", "amd64"). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above.
Can we also add these tests with PD enabled? We can run the test with secondary IP mode, enable PD, ASG can be reduced to 0 and again back to few nodes and then run the test again. |
bbb0eaf
to
6d3ef43
Compare
PD tests were added. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
* Adding Tigera operator installation, Stars resource installations and tests * Calico test pods can only work on amd64 nodes. * Need cordon the arm64 nodes to test amd version calico and stars * we should run new image CNI test and then calico tests * Enable metrics for calico test * Fix make format * updates for node label and adding PD tests
What type of PR is this?
This PR is fixing a failing weekly Calico integration tests and replacing it with helm installation with demoed STAR network policy tests suite.
bug
Which issue does this PR fix:
Fixes #1874
What does this PR do / Why do we need it:
We have deprecated support of Calico charts in this repo, but we need do routine integration tests with certain/latest calico version with our VPC CNI. This PR contains the following changes/additions:
RUN_LATEST_CALICO_VERSION == false
DEPROVISION == false
, EC2 instances will be terminated to avoid impact for future tests in the clustercalico_test_status
RUN_CALICO_TEST_WITH_PD
NOT set, the script will also run PD with Calico by defaultIf an issue # is not available please add repro steps and logs from IPAMD/CNI showing the issue:
Testing done on this change:
Using specified Calico version:
Using latest Calico released version (Automatically fetching from Calico)
Prefix Delegation tests
Automation added to e2e:
yes
Will this PR introduce any new dependencies?:
no
Will this break upgrades or downgrades. Has updating a running cluster been tested?:
no
Does this change require updates to the CNI daemonset config files to work?:
no
Does this PR introduce any user-facing change?:
no
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.