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

Made image registries configurable from registry.yaml file #60848

Merged
merged 1 commit into from Sep 25, 2018

Conversation

adelina-t
Copy link
Contributor

Make image registries used by e2e tests configurable via a registry.yaml file. File path is sent via env variable. If no such variable is set, registry names default to the ones already defined.

Fixes #60487

@k8s-ci-robot
Copy link
Contributor

@adelina-t: Adding do-not-merge/release-note-label-needed because the release note process has not been followed.

One of the following labels is required "release-note", "release-note-action-required", or "release-note-none".
Please see: https://git.k8s.io/community/contributors/devel/pull-requests.md#write-release-notes-if-needed.

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/test-infra repository.

@k8s-ci-robot
Copy link
Contributor

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.


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/test-infra repository. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Mar 6, 2018
@k8s-ci-robot k8s-ci-robot requested review from enisoc and ixdy March 6, 2018 16:04
@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Mar 6, 2018
@ixdy
Copy link
Member

ixdy commented Mar 6, 2018

can you sign the CLA, please?

@ixdy
Copy link
Member

ixdy commented Mar 6, 2018

/assign
/cc @mkumatag

@mkumatag
Copy link
Member

mkumatag commented Mar 7, 2018

@adelina-t what is the proposal here for supporting windows platform? Keeping windows docker images is some other location may not scale for a long run, IMO we should have windows images also co-located with all other images(linux).

@adelina-t
Copy link
Contributor Author

@mkumatag The ideea is to support testing on k8s clusters containing windows on Azure ( for the moment ). Since the cluster will be deployed in Azure we might benefit from locating images in a registry in Azure as well, since the windows images are fairly big.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Mar 13, 2018
@PatrickLang
Copy link
Contributor

/sig testing

@k8s-ci-robot k8s-ci-robot added the sig/testing Categorizes an issue or PR as relevant to SIG Testing. label Mar 19, 2018
@@ -0,0 +1,4 @@
e2eRegistry: atuvenie
Copy link
Member

Choose a reason for hiding this comment

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

did you intend for this file to be checked into the repo? if so, renaming this to image_registry.yaml, or placing it in the test/images directory might be a bit clearer.

additionally, this file isn't included in kubernetes-test.tar.gz, so it's not available to e2e tests currently. if you want to include it, you'll need to update build/BUILD:test-portable-targets" and hack/lib/golang.sh:KUBE_TEST_PORTABLE`.

)

type Registry struct {
Copy link
Member

Choose a reason for hiding this comment

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

minor nit: RegistryList, maybe?

SampleRegistry: "gcr.io/google-samples",
}

if os.Getenv("KUBE_TEST_REPO_LIST") == "" {
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 use a flag instead of an environment variable for this?

@ixdy
Copy link
Member

ixdy commented Mar 20, 2018

/ok-to-test

@k8s-ci-robot k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Mar 20, 2018
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 8, 2018
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 19, 2018
@adelina-t adelina-t force-pushed the configure_test_image_repo branch 2 times, most recently from ccd1842 to 5918f21 Compare June 19, 2018 14:36
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 2, 2018
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 28, 2018
@mkumatag
Copy link
Member

At the moment, the flags are being consumed during the module init phase, meaning that we could only initialize TestContext.KubeTestRepoList only during init or after, which is already too late, since plenty of tests require those registries and images to be initialized by the time their modules load.

@ixdy @dims any comments?

test/utils/image/manifest.go Outdated Show resolved Hide resolved
@ixdy
Copy link
Member

ixdy commented Sep 17, 2018

otherwise this basically LGTM

@dims
Copy link
Member

dims commented Sep 18, 2018

/release-note-none

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Sep 18, 2018
@dims
Copy link
Member

dims commented Sep 18, 2018

/lgtm

( agree with @ixdy )

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 18, 2018
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 18, 2018
@adelina-t
Copy link
Contributor Author

Small change because pull-kubernetes-verify was failing. ( was calling panic with only one param in manifest.go )

@ixdy
Copy link
Member

ixdy commented Sep 18, 2018

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 18, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adelina-t, dims, ixdy

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 Sep 18, 2018
@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

@benmoss
Copy link
Member

benmoss commented Sep 24, 2018

/retest

2 similar comments
@ixdy
Copy link
Member

ixdy commented Sep 24, 2018

/retest

@ixdy
Copy link
Member

ixdy commented Sep 25, 2018

/retest

@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Sep 25, 2018

@adelina-t: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-kubemark-e2e-gce 5918f2131708a15d93f62fe4f0ff484ade242560 link /test pull-kubernetes-kubemark-e2e-gce

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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/test-infra repository. I understand the commands that are listed here.

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

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. 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. release-note-none Denotes a PR that doesn't merit a release note. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants