Skip to content
This repository has been archived by the owner on Apr 25, 2023. It is now read-only.

Kubernetes 1.18.5 dependency upgrade (plus other upgrades) #1244

Merged
merged 15 commits into from
Jul 6, 2020

Conversation

jimmidyson
Copy link
Contributor

What this PR does / why we need it:

Upgraded dependencies for Kubernetes 1.18.5 as well as various other small upgrades. Needed to keep up to date with Kubernetes changes and improvements, especially for those projects using kubefed as a library.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):

Special notes for your reviewer:

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jul 2, 2020
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/invalid-owners-file Indicates that a PR should not merge because it has an invalid OWNERS file in it. labels Jul 2, 2020
@jimmidyson jimmidyson force-pushed the kubernetes-1.18.5 branch from 00d3951 to e0e2028 Compare July 2, 2020 19:00
@hectorj2f
Copy link
Contributor

/lgtm

@RainbowMango
Copy link
Contributor

/verify-owners

@mrbobbytables
Copy link

The repo-infra commit will need to be updated to use the version without nico in the owners file. 👍

@RainbowMango
Copy link
Contributor

Thanks Bob, I'm just curious, seems clarketm already the member of org, why robot still saying not.

@mrbobbytables
Copy link

They have to accept the org membership request first before it will return valid

@RainbowMango
Copy link
Contributor

Oh, right, that makes sense.

@RainbowMango
Copy link
Contributor

Another question comes to my mind, why we need to check OWNER files even in a vendor path?

In this case, we vendors a Kubernetes repo, that's ok we add the guys to Kubernetes org. What if we vendors another repo which not belongs to Kubernetes but have an OWNER file in it?

@mrbobbytables How do you think? Is there anything I didn't know?

@nikhita
Copy link
Member

nikhita commented Jul 3, 2020

What if we vendors another repo which not belongs to Kubernetes but have an OWNER file in it?

@RainbowMango It should ignore vendor directories and that was the behavior before. Looks like a bug that it doesn't ignore it anymore, I'll check if there were some things changed recently that could affect this behavior. For reference:

https://github.com/kubernetes/test-infra/blob/bb65d07c9ece6311ea82e58925b59ebd24f28d22/prow/config/config.go#L394

@nikhita
Copy link
Member

nikhita commented Jul 3, 2020

As a side note, should the repo-infra directory be under the path vendor/k8s.io/repo-infra instead of vendor/repo-infra?

@RainbowMango
Copy link
Contributor

As a side note, should the repo-infra directory be under the path vendor/k8s.io/repo-infra instead of vendor/repo-infra?

Awesome! Nikhita. I doubt if the vendor is the right choice, maybe a third-part path?

@jimmidyson jimmidyson force-pushed the kubernetes-1.18.5 branch from 73b1329 to 2eb2f52 Compare July 3, 2020 08:30
@k8s-ci-robot k8s-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jul 3, 2020
@jimmidyson
Copy link
Contributor Author

@RainbowMango I agree that vendor is probably not the right choice, but vendor used to be ignored for OWNERS check as @nikhita said above, hence placing it in there.

@nikhita nikhita removed the do-not-merge/invalid-owners-file Indicates that a PR should not merge because it has an invalid OWNERS file in it. label Jul 3, 2020
@nikhita
Copy link
Member

nikhita commented Jul 3, 2020

Fwiw I've now manually removed the do-not-merge/invalid-owners-file label since it appears to be a bug in prow. Will investigate but don't want it to be a blocker for y'all :)

@RainbowMango
Copy link
Contributor

@jimmidyson Thanks for your explanation, I didn't know the background before.

Will investigate but don't want it to be a blocker for y'all

@nikhita I guess Prow will check this for every PR. That means prow will set the do-not-merge/invalid-owners-file label before it gets fixed, right?
No worries, I just curious about the mechanisms of Prow.

@nikhita
Copy link
Member

nikhita commented Jul 3, 2020

That means prow will set the do-not-merge/invalid-owners-file label before it gets fixed, right?

Yes, but it's problematic only for the PRs that would touch vendor/. Works as intended in other cases. 👍

@jimmidyson jimmidyson force-pushed the kubernetes-1.18.5 branch from 2eb2f52 to 8055ba6 Compare July 3, 2020 10:00
@jimmidyson
Copy link
Contributor Author

jimmidyson commented Jul 3, 2020

From @nikhita's hint (thank you!), I've moved repo-infra from vendor/repo-infra (which was incorrect to begin with...) to vendor/k8s.io/repo-infra which should match with the ignore list in invalid owners check.

@irfanurrehman
Copy link
Contributor

Thanks for doing this @jimmidyson
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: irfanurrehman, jimmidyson

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:
  • OWNERS [irfanurrehman,jimmidyson]

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

@irfanurrehman
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 6, 2020
@k8s-ci-robot k8s-ci-robot merged commit dd7ec09 into kubernetes-retired:master Jul 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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 Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants