Update e2e test images url #103724

claudiubelu · 2021-07-15T20:37:47Z

What type of PR is this?

/kind cleanup

/sig testing
/priority important-soon

What this PR does / why we need it:

Removes any reference from the registry gcr.io/kubernetes-e2e-test-images in kubernetes/kubernetes, replacing them with k8s.gcr.io/kubernetes-e2e-test-images. In some cases, the images had to be updated since a few things have changed since
their original implementation, most notably being the fact that some of the images have been centralized into the agnhost image.

Co-Authored-By: Claudiu Belu cbelu@cloudbasesolutions.com

Which issue(s) this PR fixes:

Fixes #96770

Special notes for your reviewer:

Cherry-picked from: #100470

Does this PR introduce a user-facing change?

``gcr.io/kubernetes-e2e-test-images`` will no longer be used in E2E / CI testing, ``k8s.gcr.io/e2e-test-images`` will be used instead.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

claudiubelu · 2021-07-15T20:37:56Z

/cc @spiffxp

spiffxp · 2021-07-15T20:57:19Z

@claudiubelu this needs a release note, which I don't have the bandwidth to draft today...

If I knew the answer to this before, it's been paged out of my brain, and I'll need time to page back in... what are the implications for people who are used to mirroring/air-gapping kubernetes-e2e-test-images? Do they need to use a different invocation after this lands? Would things break if they used the same invocation as they're using for v1.21? etc.

/cc @wilsonehusin @dims
I want eyes from someone who's used to air-gapping these images, if it's not you maybe you can point to someone who would be more appropriate?

dims · 2021-07-15T22:46:55Z

@spiffxp for the air gapped scenarios that i have seen, folks just throw all the necessary images in the same bucket, it's us who have them in different buckets :)

cc @johnSchnake @vladimirvivien

spiffxp · 2021-07-16T00:18:28Z

/retest

claudiubelu · 2021-07-16T13:04:46Z

/test pull-kubernetes-integration

vladimirvivien · 2021-07-16T17:56:56Z

As far as an airgap use case for, say, Sonobuoy, this should be Ok as long as e2e test binary returns the proper repo when queried .

claudiubelu · 2021-07-19T09:24:31Z

/test pull-kubernetes-integration

smarterclayton · 2021-07-19T13:49:31Z

test/e2e/testing-manifests/serviceloadbalancer/netexecrc.yaml

@@ -12,7 +12,8 @@ spec:
    spec:
      containers:
      - name: netexec
-        image: gcr.io/kubernetes-e2e-test-images/netexec:1.0
+        image: k8s.gcr.io/e2e-test-images/agnhost:2.32


Is this file even used anymore? I couldn't find any references to it (doing a very simple search in the repo), and the interesting part of a lot of these changes is that they aren't used in e2e anymore (but a few are used in the test/cmd shell scripts). The key point is that these can't be used inside of e2e binaries because they aren't subject to dynamic replacement (there's no code reading these and then looking up the correct value of the image before running it), so it's possible that we should move these out of test/e2e and into test/cmd/ (where appropriate).

For example:

kubectl create -f test/e2e/testing-manifests/kubectl/agnhost-primary-pod.yaml "${kube_flags[@]}"

in test/cmd/core.sh is the only reference to the file above, so a separate PR moving these is probably appropriate.

I could have sworn we had code that dynamically substituted to support repo mappings passed in via KUBE_TEST_REPO

kubernetes/test/utils/image/manifest.go

Lines 253 to 255 in 9f47110

if repo := os.Getenv("KUBE_TEST_REPO"); len(repo) > 0 {

configs = GetMappedImageConfigs(originalImageConfigs, repo)

}

Maybe it's been removed since I last looked?

Ah, here it is

kubernetes/test/utils/image/manifest.go

Lines 352 to 356 in 9f47110

// ReplaceRegistryInImageURL replaces the registry in the image URL with a custom one based

// on the configured registries.

func ReplaceRegistryInImageURL(imageURL string) (string, error) {

return replaceRegistryInImageURLWithList(imageURL, registry)

}

Used by

kubernetes/test/e2e/storage/utils/create.go

Lines 626 to 629 in 9f47110

func patchContainerImages(containers []v1.Container) error {

var err error

for i, c := range containers {

containers[i].Image, err = imageutils.ReplaceRegistryInImageURL(c.Image)

Which you can get to via

kubernetes/test/e2e/storage/utils/create.go

Line 197 in 9f47110

func CreateFromManifests(f *framework.Framework, driverNamespace *v1.Namespace, patch func(item interface{}) error, files ...string) (func(), error) {

or

kubernetes/test/e2e/storage/utils/create.go

Line 116 in 9f47110

func PatchItems(f *framework.Framework, driverNamspace *v1.Namespace, items ...interface{}) error {

tl;dr I don't know if you were talking about the things in this specific file or these manifests in general, but they can have their images dynamically substituted

So, indeed, some of these files do not seem to be used anymore, like:

test/e2e/testing-manifests/ingress/http/rc.yaml test/e2e/testing-manifests/ingress/http2/rc.yaml test/e2e/testing-manifests/ingress/multiple-certs/rc.yaml test/e2e/testing-manifests/ingress/pre-shared-cert/rc.yaml test/e2e/testing-manifests/ingress/static-ip/rc.yaml test/e2e/testing-manifests/serviceloadbalancer/netexecrc.yaml

Some of them are still being used, like the test/e2e/testing-manifests/ingress/neg* ones.

test/e2e/testing-manifests/kubectl/agnhost-primary-pod.yaml seems to be used only in test/cmd/core.sh, so it could probably be moved out of test/e2e if needed.

And indeed, for some files the used images are dynamically substituted, which can be useful for airgapped environments (originally, AFAIK, the mechanism was introduced so we can use container images with Windows support for Windows test runs, before all the necessary work merged into k/k test images). But any other cleanup / change can be done in another PRs, specifically targeted towards those purposes. This PR just cleans up the gcr.io references.

fedebongio · 2021-07-20T20:07:43Z

/triage accepted

spiffxp · 2021-07-27T03:59:52Z

/milestone v1.23

Removes any reference from the registry gcr.io/kubernetes-e2e-test-images in kubernetes/kubernetes, replacing it with k8s.gcr.io/kubernetes-e2e-test-images. In some cases, the images had to be updated since a few things have changed since their original implementation, most notably being the fact that some of the images have been centralized into the agnhost image. Co-Authored-By: Claudiu Belu <cbelu@cloudbasesolutions.com>

claudiubelu · 2021-07-29T13:16:33Z

/test pull-kubernetes-integration

claudiubelu · 2021-08-04T08:01:06Z

/test pull-kubernetes-integration

claudiubelu · 2021-08-05T08:36:10Z

/test pull-kubernetes-integration

spiffxp

/approve
/lgtm

claudiubelu · 2021-08-11T07:48:58Z

@smarterclayton , it seems you're an approver for sample-apiserver. Is there anything else that needs to be addressed for this PR?

spiffxp · 2021-08-12T14:49:19Z

staging/src/k8s.io/sample-apiserver/artifacts/example/deployment.yaml

+        # docker pull k8s.gcr.io/e2e-test-images/sample-apiserver:1.17.4
+        # docker tag k8s.gcr.io/e2e-test-images/sample-apiserver:1.17.4 kube-sample-apiserver:latest


/cc @liggitt @lavalamp @deads2k
we need an /approve from someone in sample-apiserver/OWNERS for this comment change

liggitt · 2021-08-12T14:57:27Z

/approve

for comment change

k8s-ci-robot · 2021-08-12T14:58:07Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: claudiubelu, liggitt, spiffxp

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:

~~hack/testdata/OWNERS~~ [liggitt,spiffxp]
~~staging/src/k8s.io/sample-apiserver/OWNERS~~ [liggitt]
~~test/OWNERS~~ [liggitt,spiffxp]

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 requested review from aojea and cheftako July 15, 2021 20:37

k8s-ci-robot requested a review from spiffxp July 15, 2021 20:37

k8s-ci-robot requested review from dims and wilsonehusin July 15, 2021 20:57

smarterclayton reviewed Jul 19, 2021

View reviewed changes

k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jul 20, 2021

spiffxp mentioned this pull request Jul 27, 2021

Use k8s.gcr.io/e2e-test-images instead of gcr.io/kubernetes-e2e-test-images for e2e test images #96770

Closed

k8s-ci-robot added this to the v1.23 milestone Jul 27, 2021

k8s-ci-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Jul 28, 2021

k8s-ci-robot removed the do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. label Jul 28, 2021

claudiubelu force-pushed the update-e2e-test-images-url branch from c22e276 to bbb368b Compare August 4, 2021 11:09

spiffxp approved these changes Aug 5, 2021

View reviewed changes

k8s-ci-robot assigned spiffxp Aug 5, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 5, 2021

spiffxp reviewed Aug 12, 2021

View reviewed changes

k8s-ci-robot requested review from deads2k, lavalamp and liggitt August 12, 2021 14:49

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 12, 2021

spiffxp mentioned this pull request Aug 12, 2021

WIP: Update e2e test images url #100470

Closed

k8s-ci-robot merged commit 98e5263 into kubernetes:master Aug 12, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update e2e test images url #103724

Update e2e test images url #103724

claudiubelu commented Jul 15, 2021 •

edited

claudiubelu commented Jul 15, 2021

spiffxp commented Jul 15, 2021 •

edited

dims commented Jul 15, 2021

spiffxp commented Jul 16, 2021

claudiubelu commented Jul 16, 2021

vladimirvivien commented Jul 16, 2021

claudiubelu commented Jul 19, 2021

smarterclayton Jul 19, 2021

spiffxp Jul 20, 2021

spiffxp Jul 20, 2021 •

edited

spiffxp Jul 20, 2021

claudiubelu Jul 28, 2021

fedebongio commented Jul 20, 2021

spiffxp commented Jul 27, 2021

claudiubelu commented Jul 29, 2021

claudiubelu commented Aug 4, 2021

claudiubelu commented Aug 5, 2021

spiffxp left a comment

claudiubelu commented Aug 11, 2021

spiffxp Aug 12, 2021

liggitt commented Aug 12, 2021

k8s-ci-robot commented Aug 12, 2021

	if repo := os.Getenv("KUBE_TEST_REPO"); len(repo) > 0 {
	configs = GetMappedImageConfigs(originalImageConfigs, repo)
	}

	// ReplaceRegistryInImageURL replaces the registry in the image URL with a custom one based
	// on the configured registries.
	func ReplaceRegistryInImageURL(imageURL string) (string, error) {
	return replaceRegistryInImageURLWithList(imageURL, registry)
	}

	func patchContainerImages(containers []v1.Container) error {
	var err error
	for i, c := range containers {
	containers[i].Image, err = imageutils.ReplaceRegistryInImageURL(c.Image)

		# docker pull k8s.gcr.io/e2e-test-images/sample-apiserver:1.17.4
		# docker tag k8s.gcr.io/e2e-test-images/sample-apiserver:1.17.4 kube-sample-apiserver:latest

Update e2e test images url #103724

Update e2e test images url #103724

Conversation

claudiubelu commented Jul 15, 2021 • edited

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

claudiubelu commented Jul 15, 2021

spiffxp commented Jul 15, 2021 • edited

dims commented Jul 15, 2021

spiffxp commented Jul 16, 2021

claudiubelu commented Jul 16, 2021

vladimirvivien commented Jul 16, 2021

claudiubelu commented Jul 19, 2021

smarterclayton Jul 19, 2021

Choose a reason for hiding this comment

spiffxp Jul 20, 2021

Choose a reason for hiding this comment

spiffxp Jul 20, 2021 • edited

Choose a reason for hiding this comment

spiffxp Jul 20, 2021

Choose a reason for hiding this comment

claudiubelu Jul 28, 2021

Choose a reason for hiding this comment

fedebongio commented Jul 20, 2021

spiffxp commented Jul 27, 2021

claudiubelu commented Jul 29, 2021

claudiubelu commented Aug 4, 2021

claudiubelu commented Aug 5, 2021

spiffxp left a comment

Choose a reason for hiding this comment

claudiubelu commented Aug 11, 2021

spiffxp Aug 12, 2021

Choose a reason for hiding this comment

liggitt commented Aug 12, 2021

k8s-ci-robot commented Aug 12, 2021

claudiubelu commented Jul 15, 2021 •

edited

spiffxp commented Jul 15, 2021 •

edited

spiffxp Jul 20, 2021 •

edited