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

test(compatibility-versions): adds test script and periodic prow job for testing kubernetes feature compatibility versions #33534

Merged

Conversation

aaron-prindle
Copy link
Contributor

@aaron-prindle aaron-prindle commented Sep 26, 2024

This PR is a part of KEP-4330: Compatibility Versions in Kubernetes and adds the necessary components for a kubernetes compatibility versions test e2e test. The goal of this E2E test currently is to have nightly test run that uses compatibility versions (via the --emulated-versions= flag) to:

  • build the relevant kubernetes binaries @ HEAD (kube-apiserver, kube-controller-manager, and kube-scheduler)
  • set the --emulated-versions=n-1..3 (where n is the k8s version @ HEAD) for the relevant k8s components and create a cluster
  • pull down the n-1..3 release branch of k8s and run the n-1..3 E2E tests against the cluster using the k8s binaries @ HEAD w/ the –emulated-versions=n-1..3

This way we will have a smoke test around the integration and specifically able to validate that compatibility versions API enablement is as expected. Feature gate enablement correctness testing is WIP and will be part of a future PR.

NOTE: compatibility versions is only support since k8s release v1.31 and this PR reflects this opting for 1.31 hardcoding of the EMULATED_VERSION parameter and not doing any validations on that value in this first PR related to E2E testing this.

The files here are adapted from:

Prow Config Questions:

  • Currently this is an early version of the test that I am hoping to iterate on. Should I have test-grid annotations on this?

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Sep 26, 2024
@k8s-ci-robot k8s-ci-robot added the area/config Issues or PRs related to code in /config label Sep 26, 2024
@k8s-ci-robot k8s-ci-robot added area/jobs sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 26, 2024
@aaron-prindle aaron-prindle force-pushed the compatibility-versions-test branch 3 times, most recently from ef74464 to b858224 Compare September 26, 2024 05:22
Copy link
Member

@krzyzacy krzyzacy left a comment

Choose a reason for hiding this comment

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

I'll leave the bash part to @aojea ... or, can this be written in go?

periodics:
- interval: 24h
cluster: k8s-infra-prow-build
name: pull-kubernetes-e2e-kind-emulated-version
Copy link
Member

Choose a reason for hiding this comment

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

name it ci-kubernetes-e2e-* for periodic jobs, pull-kubernetes-e2e are for presubmits in general

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, renamed to:

name: ci-kubernetes-e2e-kind-compatibility-versions

testgrid-dashboards: sig-testing-kind
testgrid-tab-name: compatibility-version-test
description: Uses kubetest & kind to run e2e tests from older (n-1..3) kubernetes releases against a latest kubernetes master components w/ --emulated-version=<n-1..3> set.
testgrid-alert-email: aprindle@google.com
Copy link
Member

Choose a reason for hiding this comment

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

can we route to a rotation? (maybe ok for now, add a TODO?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, added a TODO comment for this now

Copy link
Member

Choose a reason for hiding this comment

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

In some places we have a SIG alerts list, I don't know if api machinery or architecture have one

it can route to multiple emails anyhow, so if you're personally working on this and want to be alerted to start that's common and fine

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am personally working on this and would like to be alerted to start so I think just having aprindle@google.com there for now should be ok in that case.

@aaron-prindle aaron-prindle force-pushed the compatibility-versions-test branch 4 times, most recently from cd19a98 to fb0fdcc Compare September 26, 2024 06:13
@aaron-prindle
Copy link
Contributor Author

/cc @BenTheElder

- curl -sSL https://kind.sigs.k8s.io/dl/latest/linux-amd64.tgz | tar xvfz - -C "${PATH%%:*}/" && ${GOPATH}/src/k8s.io/test-infra/experiment/compatibility-versions/e2e-k8s-compatibility-versions.sh
env:
- name: EMULATED_VERSION
value: "1.31" # TODO(aaron-prindle) FIXME - hardcoded for now
Copy link
Member

Choose a reason for hiding this comment

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

we might be able to handle this with the config-forker, it only needs to change when we branch the release anyhow

https://github.com/kubernetes/test-infra/tree/master/releng/config-forker <-- run when we introduce a new release branch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I was not aware of the config-forker. I have created #33573 to track properly parameterizing the branch/branches to test against. Currently this won't be an issue as only testing 1.31 is all that is needed for the next ~2 months and I am hoping to get something in to kick the tires on the test. Lmk if that sg, thanks!

Copy link
Member

Choose a reason for hiding this comment

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

Yeah the forker is pretty niche, I think it may be cleaner than alternatives to solve this long term though. Tracking in an issue SGTM

- wrapper.sh
- bash
- -c
- curl -sSL https://kind.sigs.k8s.io/dl/latest/linux-amd64.tgz | tar xvfz - -C "${PATH%%:*}/" && ${GOPATH}/src/k8s.io/test-infra/experiment/compatibility-versions/e2e-k8s-compatibility-versions.sh
Copy link
Member

@BenTheElder BenTheElder Oct 2, 2024

Choose a reason for hiding this comment

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

Suggested change
- curl -sSL https://kind.sigs.k8s.io/dl/latest/linux-amd64.tgz | tar xvfz - -C "${PATH%%:*}/" && ${GOPATH}/src/k8s.io/test-infra/experiment/compatibility-versions/e2e-k8s-compatibility-versions.sh
- curl -sSL https://kind.sigs.k8s.io/dl/latest/linux-amd64.tgz | tar xvfz - -C "${PATH%%:*}/" && "$(go env GOPATH)"/src/k8s.io/test-infra/experiment/compatibility-versions/e2e-k8s-compatibility-versions.sh

or simpler with precedent:

Suggested change
- curl -sSL https://kind.sigs.k8s.io/dl/latest/linux-amd64.tgz | tar xvfz - -C "${PATH%%:*}/" && ${GOPATH}/src/k8s.io/test-infra/experiment/compatibility-versions/e2e-k8s-compatibility-versions.sh
- curl -sSL https://kind.sigs.k8s.io/dl/latest/linux-amd64.tgz | tar xvfz - -C "${PATH%%:*}/" && ./../test-infra/experiment/compatibility-versions/e2e-k8s-compatibility-versions.sh

Copy link
Contributor Author

@aaron-prindle aaron-prindle Oct 3, 2024

Choose a reason for hiding this comment

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

Did you have a preference from the above options here? I used ${GOPATH} initially as it was in config/jobs/containerd/containerd/containerd-presubmit-jobs.yaml - https://github.com/kubernetes/test-infra/blob/master/config/jobs/containerd/containerd/containerd-presubmit-jobs.yaml#L261. I've changed this to use the first recommendation "$(go env GOPATH)" for now

Copy link
Member

Choose a reason for hiding this comment

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

Sorry for the late reply: I prefer the second because locally the user may not have a GOPATH these days but they're very likely to have these clones side by side (or they don't have them at all, and this won't work regardless) but it doesn't matter much.

Copy link
Member

@BenTheElder BenTheElder left a comment

Choose a reason for hiding this comment

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

cc @aojea

LGTM, I think, any comments antonio?

Comment on lines +150 to +151
feature_gates='{"AllAlpha":false,"AllBeta":false}'
runtime_config='{"api/alpha":"false", "api/beta":"false"}'
Copy link
Member

Choose a reason for hiding this comment

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

do you want to run with all betas enable?

Copy link
Contributor Author

@aaron-prindle aaron-prindle Oct 4, 2024

Choose a reason for hiding this comment

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

Good point, thanks for catching this. I would like to run with all betas enable. The script there is essentially https://github.com/kubernetes-sigs/kind/blob/main/hack/ci/e2e-k8s.sh with the only addition being the --emulated-version flag plumbed through. I have updated the script directly to now have true for both Beta feature and API enablement:

    feature_gates='{"AllAlpha":false,"AllBeta":true}'
    runtime_config='{"api/alpha":"false", "api/beta":"true"}'

EDIT: See below, I have changed the approach of how to do this

Copy link
Contributor Author

@aaron-prindle aaron-prindle Oct 4, 2024

Choose a reason for hiding this comment

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

I realize now the script is parameterized to allow for the following. I undid my direct script changes (see above comment) and updated the prow job to pass in the params to the script to enable betas:

      - name: FEATURE_GATES
        value: '{"AllBeta":true}'
      - name: RUNTIME_CONFIG
        value: '{"api/beta":"true", "api/ga":"true"}'

Copy link
Member

Choose a reason for hiding this comment

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

Quick question: Why are we running with Beta APIs and feature gates enabled? Shouldn't we use the default feature gate flag at a particular version?

@aaron-prindle aaron-prindle force-pushed the compatibility-versions-test branch 2 times, most recently from 21c3949 to abdbf78 Compare October 4, 2024 17:50
…for testing kubernetes feature compatibility versions
@aojea
Copy link
Member

aojea commented Oct 7, 2024

/lgtm

Think about having a presubmit optional and disabled by default, use to be helpfor for debugging or reviewing code related to your feature

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 7, 2024
@aojea
Copy link
Member

aojea commented Oct 7, 2024

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aaron-prindle, aojea

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 Oct 7, 2024
@k8s-ci-robot k8s-ci-robot merged commit f3ec85a into kubernetes:master Oct 7, 2024
7 checks passed
@k8s-ci-robot
Copy link
Contributor

@aaron-prindle: Updated the job-config configmap in namespace default at cluster test-infra-trusted using the following files:

  • key compatibility-versions-e2e.yaml using file config/jobs/kubernetes/sig-testing/compatibility-versions-e2e.yaml

In response to this:

This PR is a part of KEP-4330: Compatibility Versions in Kubernetes and adds the necessary components for a kubernetes compatibility versions test e2e test. The goal of this E2E test currently is to have nightly test run that uses compatibility versions (via the --emulated-versions= flag) to:

  • build the relevant kubernetes binaries @ HEAD (kube-apiserver, kube-controller-manager, and kube-scheduler)
  • set the --emulated-versions=n-1..3 (where n is the k8s version @ HEAD) for the relevant k8s components and create a cluster
  • pull down the n-1..3 release branch of k8s and run the n-1..3 E2E tests against the cluster using the k8s binaries @ HEAD w/ the –emulated-versions=n-1..3

This way we will have a smoke test around the integration and specifically able to validate that compatibility versions API enablement is as expected. Feature gate enablement correctness testing is WIP and will be part of a future PR.

NOTE: compatibility versions is only support since k8s release v1.31 and this PR reflects this opting for 1.31 hardcoding of the EMULATED_VERSION parameter and not doing any validations on that value in this first PR related to E2E testing this.

The files here are adapted from:

Prow Config Questions:

  • Currently this is an early version of the test that I am hoping to iterate on. Should I have test-grid annotations on this?

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@BenTheElder
Copy link
Member

Think about having a presubmit optional and disabled by default, use to be helpfor for debugging or reviewing code related to your feature

we're going with iterating towards a release-informing => blocking job for now.

decorate: true
decoration_config:
timeout: 60m
extra_refs:
Copy link
Member

@BenTheElder BenTheElder Oct 7, 2024

Choose a reason for hiding this comment

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

oh, because this is interval based, we need to explicitly add cloning kubernetes as well (first which will make it default for being the working directory)

(or switch to have the script use kubernetes from a CI build instead of building itself)

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/config Issues or PRs related to code in /config area/jobs 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. sig/testing Categorizes an issue or PR as relevant to SIG Testing. 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.

6 participants