Skip to content

Improve vendor verification works for each staging repo#114952

Merged
k8s-ci-robot merged 1 commit intokubernetes:masterfrom
liggitt:verify-vendor-tidy
Jan 10, 2023
Merged

Improve vendor verification works for each staging repo#114952
k8s-ci-robot merged 1 commit intokubernetes:masterfrom
liggitt:verify-vendor-tidy

Conversation

@liggitt
Copy link
Copy Markdown
Member

@liggitt liggitt commented Jan 10, 2023 •
edited
Loading

What type of PR is this?

/kind bug

What this PR does / why we need it:

Adds a check to presubmit to ensure we don't merge vendor updates that break tidy when run in isolation on a staging repo with a clean go cache. This has bitten us in post-merge with ambiguous module versions / imports before.

This passes on master, and fails like the following on a PR that made staging references ambiguous:

Tidying k8s.io/api...
Tidying k8s.io/apiextensions-apiserver...
k8s.io/apiextensions-apiserver/pkg/apiserver imports
	k8s.io/apiserver/pkg/registry/generic imports
	k8s.io/apiserver/pkg/storage/storagebackend/factory imports
	go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc tested by
	go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc.test imports
	google.golang.org/grpc/interop imports
	golang.org/x/oauth2/google imports
	cloud.google.com/go/compute/metadata: ambiguous import: found package cloud.google.com/go/compute/metadata in multiple modules:
	cloud.google.com/go v0.65.0 (/Users/liggitt/go/pkg/mod/cloud.google.com/[email protected]/compute/metadata)
	cloud.google.com/go/compute v1.7.0 (/Users/liggitt/go/pkg/mod/cloud.google.com/go/[email protected]/metadata)

cc @dims @nikhita

NONE

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/bug Categorizes issue or PR as related to a bug. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 10, 2023
@liggitt
Copy link
Copy Markdown
Member Author

liggitt commented Jan 10, 2023

/sig architecture
/area code-organization

@k8s-ci-robot k8s-ci-robot added sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. area/code-organization Issues or PRs related to kubernetes code organization approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jan 10, 2023
nikhita
nikhita approved these changes Jan 10, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@nikhita nikhita left a comment •
edited
Loading

Choose a reason for hiding this comment

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

/lgtm

This will make the script run longer, right? I'm guessing it won't be too long though..not enough to be problematic.

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 10, 2023
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Jan 10, 2023

LGTM label has been added.

DetailsGit tree hash: d1d363ba7311bf8105ec2d55013d98469d22542d

@dims
Copy link
Copy Markdown
Member

dims commented Jan 10, 2023

/approve
/lgtm

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Jan 10, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dims, liggitt, nikhita

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details 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

@liggitt
Copy link
Copy Markdown
Member Author

liggitt commented Jan 10, 2023

This will make the script run longer, right? I'm guessing it won't be too long though..not enough to be problematic.

it will... that's why I put it in verify-vendor instead of update-vendor

practically, that presubmit is among our shorter jobs, so I'm not that concerned about it

@liggitt
Copy link
Copy Markdown
Member Author

liggitt commented Jan 10, 2023

/triage accepted
/priority important-soon

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jan 10, 2023
@liggitt
Copy link
Copy Markdown
Member Author

liggitt commented Jan 10, 2023 •
edited
Loading

... e2e unit tests (?) failed in https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/114952/pull-kubernetes-unit/1612836634175213568 ...

k8s.io/kubernetes/test/e2e/framework/internal/unittests/cleanup: TestCleanup

opened #114954 to improve message clipping in that case

@liggitt
Copy link
Copy Markdown
Member Author

liggitt commented Jan 10, 2023

/retest

@k8s-ci-robot k8s-ci-robot merged commit ef2ef15 into kubernetes:master Jan 10, 2023
@k8s-ci-robot k8s-ci-robot added this to the v1.27 milestone Jan 10, 2023
This was referenced Jan 10, 2023
Merged
Merged
Merged
Merged
@liggitt
Copy link
Copy Markdown
Member Author

liggitt commented Jan 10, 2023

opened picks to release branches to verify this there as well

This was referenced Jan 10, 2023
Closed
Merged
Merged
Merged
Closed
k8s-ci-robot added a commit that referenced this pull request Jan 11, 2023
@k8s-ci-robot
Merge pull request #114962 from liggitt/automated-cherry-pick-of-#114…
f4f355d 
…952-upstream-release-1.24

Automated cherry pick of #114952: Improve vendor verification works for each staging repo
k8s-ci-robot added a commit that referenced this pull request Jan 11, 2023
@k8s-ci-robot
Merge pull request #114963 from liggitt/automated-cherry-pick-of-#114…
3a9dfb8 
…952-upstream-release-1.23

Automated cherry pick of #114952: Improve vendor verification works for each staging repo
k8s-ci-robot added a commit that referenced this pull request Jan 11, 2023
@k8s-ci-robot
Merge pull request #114960 from liggitt/automated-cherry-pick-of-#114…
e8d0c3b 
…952-upstream-release-1.26

Automated cherry pick of #114952: Improve vendor verification works for each staging repo
k8s-ci-robot added a commit that referenced this pull request Jan 11, 2023
@k8s-ci-robot
Merge pull request #114961 from liggitt/automated-cherry-pick-of-#114…
9da8d26 
…952-upstream-release-1.25

Automated cherry pick of #114952: Improve vendor verification works for each staging repo
@liggitt liggitt deleted the verify-vendor-tidy branch May 9, 2023 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nikhita nikhita nikhita approved these changes

@thockin thockin Awaiting requested review from thockin

@wojtek-t wojtek-t Awaiting requested review from wojtek-t

Assignees

@dims dims

@nikhita nikhita

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/code-organization Issues or PRs related to kubernetes code organization cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note-none Denotes a PR that doesn't merit a release note. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Projects

None yet

Milestone

v1.27

Development

Successfully merging this pull request may close these issues.

4 participants

@liggitt @k8s-ci-robot @dims @nikhita