fix range copy issue #87777

dbenoit17 · 2020-02-03T17:15:02Z

What type of PR is this?

/kind failing-test

What this PR does / why we need it:
The patchContainerImages function in the e2e test suite is intended to allow users to override some of the default e2e test image registries. Currently, this function is a no-op due to golang's copy semantics on ranges.

This PR alters the function to mutate the range elements directly.

Which issue(s) this PR fixes:
N/A

Special notes for your reviewer:
This function was located elsewhere in 1.16.

Does this PR introduce a user-facing change?:

NONE

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

k8s-ci-robot · 2020-02-03T17:15:06Z

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.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
If you have done the above and are still having issues with the CLA being reported as unsigned, please log a ticket with the Linux Foundation Helpdesk: https://support.linuxfoundation.org/
Should you encounter any issues with the Linux Foundation Helpdesk, send a message to the backup e-mail support address at: login-issues@jira.linuxfoundation.org

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 · 2020-02-03T17:15:10Z

Welcome @dbenoit17!

It looks like this is your first PR to kubernetes/kubernetes 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/kubernetes has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2020-02-03T17:15:11Z

Hi @dbenoit17. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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

jsafrane · 2020-02-04T13:25:32Z

/ok-to-test
Nice catch!

mkumatag · 2020-02-13T17:27:25Z

/retest

mkumatag · 2020-02-13T19:30:22Z

/retest

RA489 · 2020-03-06T06:47:34Z

/check-cla
/test pull-kubernetes-e2e-gce-csi-serial

dbenoit17 · 2020-03-06T18:02:43Z

This should be fixed. Could you try running the cla-check again?

mkumatag · 2020-03-06T18:34:21Z

/lgtm

mkumatag · 2020-03-06T18:37:30Z

@BenTheElder do you mind approving this PR?

mkumatag · 2020-03-06T18:39:07Z

This should be fixed. Could you try running the cla-check again?

@dbenoit17 lets see if we can fix in 1.16 and 1.17 branches as well?

mkumatag · 2020-03-06T18:41:39Z

/priority important-soon
/milestone v1.18
/kind bug

k8s-ci-robot · 2020-03-06T18:41:41Z

@mkumatag: You must be a member of the kubernetes/milestone-maintainers GitHub team to set the milestone. If you believe you should be able to issue the /milestone command, please contact your and have them propose you as an additional delegate for this responsibility.

In response to this:

/priority important-soon
/milestone v1.18
/kind bug

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.

mkumatag · 2020-03-06T18:43:02Z

@dims can I get this into 1.18 release? this was blocking us to override one of the image repositories while running the tests.

BenTheElder

thanks for fixing this!

BenTheElder · 2020-03-06T18:43:20Z

test/e2e/storage/utils/create.go

-	for _, c := range containers {
-		c.Image, err = imageutils.ReplaceRegistryInImageURL(c.Image)
+	for i, c := range containers {
+		containers[i].Image, err = imageutils.ReplaceRegistryInImageURL(c.Image)


ugh, we should really get a linter catching these.

BenTheElder · 2020-03-06T18:43:41Z

/lgtm
/approve

k8s-ci-robot · 2020-03-06T18:44:32Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: BenTheElder, dbenoit17

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:

~~test/e2e/storage/OWNERS~~ [BenTheElder]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

dims · 2020-03-06T18:57:52Z

/milestone v1.18

This is clearly a bug that needs fixing'

fejta-bot · 2020-03-06T22:51:27Z

/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 or /hold comment for consistent failures.

mkumatag · 2020-03-07T03:19:47Z

/retest

mkumatag · 2020-03-07T05:40:28Z

/retest

fejta-bot · 2020-03-07T11:27:28Z

/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 or /hold comment for consistent failures.

mkumatag · 2020-03-07T13:55:39Z

/retest

fix range copy issue

087dc49

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Feb 3, 2020

k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Feb 3, 2020

k8s-ci-robot added the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Feb 3, 2020

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 Feb 3, 2020

k8s-ci-robot requested review from jingxu97 and jsafrane February 3, 2020 17:16

k8s-ci-robot added area/test sig/storage Categorizes an issue or PR as relevant to SIG Storage. sig/testing Categorizes an issue or PR as relevant to SIG Testing. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Feb 3, 2020

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 4, 2020

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 6, 2020

k8s-ci-robot assigned mkumatag Mar 6, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 6, 2020

k8s-ci-robot added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Mar 6, 2020

k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Mar 6, 2020

BenTheElder reviewed Mar 6, 2020

View reviewed changes

k8s-ci-robot assigned BenTheElder Mar 6, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 6, 2020

k8s-ci-robot added this to the v1.18 milestone Mar 6, 2020

k8s-ci-robot merged commit 672aa55 into kubernetes:master Mar 7, 2020

oomichi mentioned this pull request Jul 9, 2020

Fix range copy issue #92935

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix range copy issue #87777

fix range copy issue #87777

dbenoit17 commented Feb 3, 2020 •

edited

k8s-ci-robot commented Feb 3, 2020

k8s-ci-robot commented Feb 3, 2020

k8s-ci-robot commented Feb 3, 2020

jsafrane commented Feb 4, 2020

mkumatag commented Feb 13, 2020

mkumatag commented Feb 13, 2020

RA489 commented Mar 6, 2020

dbenoit17 commented Mar 6, 2020

mkumatag commented Mar 6, 2020

mkumatag commented Mar 6, 2020

mkumatag commented Mar 6, 2020

mkumatag commented Mar 6, 2020

k8s-ci-robot commented Mar 6, 2020

mkumatag commented Mar 6, 2020

BenTheElder left a comment

BenTheElder Mar 6, 2020

BenTheElder commented Mar 6, 2020

k8s-ci-robot commented Mar 6, 2020

dims commented Mar 6, 2020

fejta-bot commented Mar 6, 2020

mkumatag commented Mar 7, 2020

mkumatag commented Mar 7, 2020

fejta-bot commented Mar 7, 2020

mkumatag commented Mar 7, 2020

fix range copy issue #87777

fix range copy issue #87777

Conversation

dbenoit17 commented Feb 3, 2020 • edited

k8s-ci-robot commented Feb 3, 2020

k8s-ci-robot commented Feb 3, 2020

k8s-ci-robot commented Feb 3, 2020

jsafrane commented Feb 4, 2020

mkumatag commented Feb 13, 2020

mkumatag commented Feb 13, 2020

RA489 commented Mar 6, 2020

dbenoit17 commented Mar 6, 2020

mkumatag commented Mar 6, 2020

mkumatag commented Mar 6, 2020

mkumatag commented Mar 6, 2020

mkumatag commented Mar 6, 2020

k8s-ci-robot commented Mar 6, 2020

mkumatag commented Mar 6, 2020

BenTheElder left a comment

Choose a reason for hiding this comment

BenTheElder Mar 6, 2020

Choose a reason for hiding this comment

BenTheElder commented Mar 6, 2020

k8s-ci-robot commented Mar 6, 2020

dims commented Mar 6, 2020

fejta-bot commented Mar 6, 2020

mkumatag commented Mar 7, 2020

mkumatag commented Mar 7, 2020

fejta-bot commented Mar 7, 2020

mkumatag commented Mar 7, 2020

dbenoit17 commented Feb 3, 2020 •

edited