✨ envtest: search the assets index for latest of a release series #3280
Conversation
The former is no longer maintained and the latter is designed to understand Kubernetes versions. This eliminates one direct dependency introduced in b04d5fd. Signed-off-by: Chris Bandy <[email protected]>
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
needs-ok-to-test
|
Hi @cbandy. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
k8s-ci-robot
added
the
size/L
cbandy
changed the title
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
alvaroaleman
added
the
tide/merge-method-squash
cbandy
force-pushed
the
envtest-versions
branch
from
0c4144d to
f96e891
Compare
Signed-off-by: Chris Bandy <[email protected]>
cbandy
force-pushed
the
envtest-versions
branch
from
f96e891 to
ffdc92b
Compare
There was a problem hiding this comment.
/hold
lgtm, leaving a hold so @sbueringer can have a look as well
k8s-ci-robot
added
the
do-not-merge/hold
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: 2191d46f9db33c664428bf6c032156ef855ef395
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alvaroaleman, cbandy 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 |
k8s-ci-robot
added
the
approved
| // Use the canonical form of the version from here on. | ||
| binaryAssetsVersion = "v" + exact | ||
| } else if binaryAssetsVersion == "" || major != 0 || minor != 0 { | ||
| // Select a stable version from the release index before continuing. | ||
| var err error | ||
| binaryAssetsIndex, err = getIndex(ctx, binaryAssetsIndexURL) | ||
| if err != nil { | ||
| return "", "", "", err | ||
| } |
There was a problem hiding this comment.
Should we add an else branch here that returns to cover error cases?
| expectSeriesMajor uint | ||
| expectSeriesMinor uint | ||
| }{ | ||
| { |
There was a problem hiding this comment.
Let's add a test case for the empty string
This allows configuring the
DownloadBinaryAssetsVersionfield ofenvtest.Environmentwith only the Major.Minor portions of a Kubernetes API version. In that case, the framework searches the asset index for the latest stable version in that release series.This matches the behavior of
setup-envtest use {major}.{minor}without the full complexity of version selectors. The leadingvis now optional, too. For example:"v1.29"resolves to v1.29.4"1.27"resolves to v1.27.1"1.24"resolves to v1.24.2Setting the field to Major.Minor.Patch or Major.Minor.Patch-PreRelease behaves the same as before:
This is an alternative implementation of #3260. It uses the k8s.io/apimachinery/pkg/util/version package and basic comparisons, rather than an external package with range/constraint matchers.