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

Switch ginkgo upgrade testing to upgrade from v1.10->latest #16483

Merged
merged 2 commits into from Jun 15, 2021

Conversation

joestringer
Copy link
Member

  • test: Bump Cilium upgrade for v1.11 development
  • test: Remove deprecated upgrade test logic

For more details, see the individual commits.

@joestringer joestringer requested a review from a team June 8, 2021 23:30
@joestringer joestringer requested a review from a team as a code owner June 8, 2021 23:30
@joestringer joestringer added the release-note/ci This PR makes changes to the CI. label Jun 8, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jun 8, 2021
@joestringer joestringer requested a review from jibi June 8, 2021 23:30
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jun 8, 2021
@joestringer joestringer requested a review from pchaigno June 8, 2021 23:30
@joestringer
Copy link
Member Author

joestringer commented Jun 8, 2021

test-only --focus="K8sUpdates"

EDIT: Initially fixed the main install but missed the cleanup steps which also need to be provided the CI repository. Fixing...

Comment on lines 246 to 248
"image.repository": "quay.io/cilium/cilium-ci",
"operator.image.repository": "quay.io/cilium/operator-ci",
"hubble.relay.image.repository": "quay.io/cilium/hubble-relay-ci",
Copy link
Member

Choose a reason for hiding this comment

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

These options are specific to v1.10. There isn't a v1.9 for cilium-ci for example.

Copy link
Member Author

Choose a reason for hiding this comment

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

Why does this matter? This PR is about master (v1.10 -> v1.11).

Copy link
Member

Choose a reason for hiding this comment

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

Don't we run nightly jobs that tests the combination of all upgrades? v1.8->v1.9, v1.9-v1.10, etc?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, do those run using the master version of the test? I'm not at all familiar with these.

Are those somehow triggered from one of the below paths? I see only policies nigthly tests, not upgrade ones:

$ git grep -i Nightly test/
test/Makefile:nightly:
test/Makefile:  ginkgo --focus "Nightly" -v -- --cilium.provision=$(PROVISION) --cilium.registryCredentials=$(REGISTRY_CREDENTIALS)
test/config/config.go:          "Specifies scope of test to be ran (k8s, Nightly, runtime)")
test/helpers/cons.go:// NightlyStableUpgradesFrom maps the cilium image versions to the helm charts
test/helpers/cons.go:// that will be used to run update tests in the Nightly test.
test/helpers/cons.go:var NightlyStableUpgradesFrom = map[string]string{
test/helpers/kubectl.go:        // Sometimes Kubectl returns 126 exit code, It use to happen in Nightly
test/helpers/scope.go:  case strings.Contains(focusString, "nightly"):
test/helpers/scope.go:          // Nightly tests run in a Kubernetes environment.
test/k8sT/PoliciesNightly.go:var _ = Describe("NightlyPolicies", func() {
test/k8sT/StressPolicy.go:var _ = Describe("NightlyPolicyStress", func() {
test/test_suite_test.go:                        // For Nightly test we need to have more than two kubernetes nodes. If

Who can I ask / how can I find out more about this?

Copy link
Member

Choose a reason for hiding this comment

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

Don't we run nightly jobs that tests the combination of all upgrades? v1.8->v1.9, v1.9-v1.10, etc?

I can't find any such thing in https://jenkins.cilium.io. We simply run this same K8sUpdates test regularly on master as part of the usual scheduled CI runs.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd imagine we regularly run each upgrade as part of the scheduled CI runs for each branch, but that would use the upgrade tests code from each individual branch to upgrade from the prior release.

@aanm if you have more details, I can look into what other changes might be required but for now I don't see how this can break other jobs.

Copy link
Member

Choose a reason for hiding this comment

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

The code changes look good to me as it is. However I'll ping @nebril and he will have the definitive answer to what happened to the Nightly tests.

@joestringer joestringer force-pushed the submit/fix-upgrade-for-v1.11 branch from 89d8667 to 15a3f4b Compare June 9, 2021 00:36
@joestringer
Copy link
Member Author

joestringer commented Jun 9, 2021

test-only --focus="K8sUpdates"

EDIT: Messed up the operator image tag name, "operator-ci-generic-ci". Fixing...

During v1.10 development, we split the docker repositories used for
official releases (cilium/*) vs. CI (cilium/*-ci). As a result, we can
no longer use the official cilium/cilium repository to perform upgrade
testing from the tip of the v1.10 branch up to newer versions (as we
would want to during the v1.11 development cycle).

Update the docker repository to instead use the CI repository so we can
pick up automatically compiled builds from the tip of the v1.10 branch,
and update the version strings used to figure out the Helm chart
versions and Cilium image versions.

This will use the most recent Helm chart from the previous branch, as
released at the time of running the test (ie if v1.9.8 is the most
recent, it will use the v1.9.8 helm charts) along with the image built
from the tip of the previous branch (ie images built directly from the
v1.9 git branch, not v1.9.8 as built from the git tag).

Signed-off-by: Joe Stringer <joe@cilium.io>
This test code was used to manage changes in helm charts from prior
releases, but it's no longer applicable for the v1.10 -> v1.11-dev
upgrade testing. Remove the unnecessary code.

Signed-off-by: Joe Stringer <joe@cilium.io>
@joestringer joestringer force-pushed the submit/fix-upgrade-for-v1.11 branch from 15a3f4b to 6fa746d Compare June 9, 2021 01:46
@joestringer
Copy link
Member Author

test-only --focus="K8sUpdates"

Copy link
Member

@pchaigno pchaigno left a comment

Choose a reason for hiding this comment

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

LGTM.

(We probably need to run the full test suite since the focused run only covers one kernel version and one K8s version.)

test/k8sT/Updates.go Show resolved Hide resolved
@joestringer
Copy link
Member Author

test-me-please

@aanm aanm requested review from nebril and removed request for jibi June 14, 2021 23:04
@aanm aanm unassigned jibi Jun 14, 2021
@aanm aanm assigned nebril and unassigned nebril Jun 14, 2021
@pchaigno
Copy link
Member

pchaigno commented Jun 15, 2021

Discussed offline with André: we will ping @nebril on Slack but don't need to wait for his answer to merge this.

@pchaigno pchaigno added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 15, 2021
@aditighag aditighag merged commit ad1e0c4 into cilium:master Jun 15, 2021
@joestringer joestringer deleted the submit/fix-upgrade-for-v1.11 branch September 1, 2021 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/ci This PR makes changes to the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants