-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
fix: make compatibility versions test use dynamic n-1 branch for test (from hardcoded release-1.31) #33837
base: master
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: aaron-prindle The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/cc @BenTheElder |
# Clone the specific Kubernetes release branch | ||
clone_kubernetes_release | ||
# Clone the previous versions Kubernetes release branch | ||
# TODO(aaron-prindle) extend the branches to test from n-1 -> n-1..3 as more k8s releases are done that support compatibility versions |
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 should also think about which branches we're trying to test, if what we're trying to test is the api-server compatibility in master
, it might better suit us to use the latest stable tagged patch release in the previous branch for the other components. In practice there's not usually a big distinction anyhow though.
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.
Also, if we did that we could fetch binaries instead of compiling both branches from source.
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.
Makes sense, given that it doesn't seem to big a distinction atm perhaps we can keep this as is for now? I created #33594 to track using precompiled binaries so perhaps we can address that in a seperate PR.
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.
Yeah, I just wanted to point that out, with the precompiled builds we have a bunch of different marker files for the subtle variations depending on what you're actually focusing on testing and how stable you want the other end of the skew.
For now 🤷, let's just get it working, but later we'll want to decide.
git clone --single-branch --branch "${KUBE_RELEASE_BRANCH}" https://github.com/kubernetes/kubernetes.git | ||
get_latest_release_version() { | ||
# Fetch all branch names | ||
git ls-remote --heads https://github.com/kubernetes/kubernetes.git | \ |
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 should probably prefer using local git state?
we can list branches or tags locally and determine this, and it will work in offline development
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.
actually, further, to make this work with N branches, it should look at something like git describe
, get the current release branch from that and then compute the N-... branch.
Because we'll want to be running this on each release going forward and those can't just assume the target is the latest release at all.
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.
Does the k8s repo that Prow pulls have all of the other branches available? If so, definitely +1 - just want to confirm that. We pass in base_ref: master
as part of the k8s/k8s repo config so wasn't sure if all branches were pulled down.
# Clone the specific Kubernetes release branch | ||
clone_kubernetes_release | ||
# Clone the previous versions Kubernetes release branch | ||
# TODO(aaron-prindle) extend the branches to test from n-1 -> n-1..3 as more k8s releases are done that support compatibility versions |
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.
Also, if we did that we could fetch binaries instead of compiling both branches from source.
# Clone the previous versions Kubernetes release branch | ||
# TODO(aaron-prindle) extend the branches to test from n-1 -> n-1..3 as more k8s releases are done that support compatibility versions | ||
export PREV_RELEASE_BRANCH="release-${EMULATED_VERSION}" | ||
git clone --single-branch --branch "${PREV_RELEASE_BRANCH}" https://github.com/kubernetes/kubernetes.git "${PREV_RELEASE_BRANCH}" |
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.
Do we really need another full clone? (also if we're going to clone, we should add some performance related flags, cloning kubernetes in full is really expensive with the 10 years of history)
If we need to use git, let's use a worktree. But I think we can fetch binaries for the old release and for the new release we should already have the checkout in order, presumably.
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.
+1, I'm familiar with some of the flags to pull branch with minimal history (--depth=1
, --single-branch) which I've changed this to use now. Did you have any other suggestions for flags there?
Can you explain how a worktree would help in this context or give an example for what you are suggesting with If we need to use git, let's use a worktree.
? Thanks
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.
+1, I'm familiar with some of the flags to pull branch with minimal history (--depth=1, --single-branch) which I've changed this to use now. Did you have any other suggestions for flags there?
--depth=1
will break git describe
(it has to walk back to find the last tag), in prow we use --filter=blob:none
instead
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.
Ah got it, I've updated this to use --filter=blob:none
now:
git clone --filter=blob:none --single-branch --branch "${PREV_RELEASE_BRANCH}" https://github.com/kubernetes/kubernetes.git "${PREV_RELEASE_BRANCH}"
TLDR I think:
|
40b1238
to
7ff6885
Compare
Currently the PR is focused on fixing #33573. I've created #33849 to track the issue with the kubelet version being incorrect. I will address the below points in a separate PR to fix #33849 if that is ok. Can also do them both in the PR, currently my plan is to do them in 2 PRs though.
|
7ff6885
to
b6498a0
Compare
… (from hardcoded release-1.31)
b6498a0
to
a396d12
Compare
Fixes #33573
This PR modifies
experiment/compatibility-versions/e2e-k8s-compatibility-versions.sh
to fetch the latest release-X.Y branch from https://github.com/kubernetes/kubernetes.git and use that as the n-1 version for compatibility versions testing. This way this test is dynamic and updates the n-1 version as new releases are made - updating automatically over time vs the current hardcodedrelease-1.31
value currently present in the test.