Bump k8s.io/kube-openapi to commit ee342a809c29

[ulucinar]

What type of PR is this?

/kind bug

What this PR does / why we need it:

Updates kube-openapi dependency to consume the aggregated OpenAPI spec lazy-marshaling behavior introduced with:
kubernetes/kube-openapi#251

Looks like lazy-marshaling introduced with kubernetes/kube-openapi#251 enables us to better scale in the #-of-CRDs-per-cluster dimension.

Which issue(s) this PR fixes:

Fixes #101755
Fixes #105932

Does this PR introduce a user-facing change?

NONE

[k8s-ci-robot]

Welcome @ulucinar!

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]

Hi @ulucinar. 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.

[ulucinar]

Hi @DangerOnTheRanger, @apelisse,
I have opened this PR to stir up some discussions on consuming @DangerOnTheRanger's PR and to plan about cherry-picks to active release branches.

I'd also like to gain experience on contributing to upstream Kubernetes so any guidance is very much appreciated :)

[DangerOnTheRanger]

/ok-to-test

[apelisse]

We have quite a few things to get from kube-openapi before the release, I'm not opposed to merging this, but we'll have to bump it again soon anyway.

[ulucinar]

I have run the following command to see whether the failing TestFlowControlSignal/upgrade_request_performed also fails in my development environment. It looks like it's passing:

$ make test WHAT=./vendor/k8s.io/kube-aggregator/pkg/apiserver GOFLAGS="-v" KUBE_TEST_ARGS='-run ^TestFlowControlSignal$'

+++ [1105 17:31:04] Running tests without code coverage and with -race
=== RUN   TestFlowControlSignal
=== RUN   TestFlowControlSignal/local
=== RUN   TestFlowControlSignal/unavailable
=== RUN   TestFlowControlSignal/request_performed
=== RUN   TestFlowControlSignal/upgrade_request_performed
--- PASS: TestFlowControlSignal (0.02s)
    --- PASS: TestFlowControlSignal/local (0.00s)
    --- PASS: TestFlowControlSignal/unavailable (0.00s)
    --- PASS: TestFlowControlSignal/request_performed (0.01s)
    --- PASS: TestFlowControlSignal/upgrade_request_performed (0.00s)
PASS
ok      k8s.io/kubernetes/vendor/k8s.io/kube-aggregator/pkg/apiserver   0.181s

Trying the bot's suggestion:
/test pull-kubernetes-unit

[ulucinar]

Hi @apelisse,
Thanks for the feedback. I think this is for the 1.23 release planned for December 7, 2021.

How do we move on with the cherry-picks to the active release branches 1.20-22? How is kube-openapi consumed in a monthly patch release? If I'm not mistaken, the deadline for the cherry-picks is November 12, 2021. Is a cherry-pick to respective release branches (release-0.20, release-0.21, release-0.22) done after this PR (or one of the upcoming kube-openapi bump PRs) are merged)? Is it possible to independently bump kube-openapi version in active release branches without this one being merged? Wouldn't it be problematic to try to consume latest kube-openapi in those release branches?

Sorry for asking lots of questions. I'm trying to better understand our plan.

[ulucinar]

Squashed the commits into a single one. Again looks like a unit test has failed. Gave it a try in my development environment:

$ make test WHAT=./vendor/k8s.io/client-go/metadata/metadatainformer GOFLAGS="-v" KUBE_TEST_ARGS='-run ^TestMetadataSharedInformerFactory$'

+++ [1108 13:08:16] Running tests without code coverage and with -race
=== RUN   TestMetadataSharedInformerFactory
=== RUN   TestMetadataSharedInformerFactory/scenario_1:_test_if_adding_an_object_triggers_AddFunc
W1108 13:08:21.210587 2507679 mutation_detector.go:53] Mutation detector is enabled, this will result in memory leakage.
I1108 13:08:21.213720 2507679 reflector.go:219] Starting reflector *v1.PartialObjectMetadata (0s) from k8s.io/client-go/metadata/metadatainformer/informer.go:90
I1108 13:08:21.213806 2507679 reflector.go:255] Listing and watching *v1.PartialObjectMetadata from k8s.io/client-go/metadata/metadatainformer/informer.go:90
I1108 13:08:21.312107 2507679 shared_informer.go:270] caches populated
I1108 13:08:21.312742 2507679 watch.go:183] Stopping fake watcher.
=== RUN   TestMetadataSharedInformerFactory/scenario_2:_tests_if_updating_an_object_triggers_UpdateFunc
I1108 13:08:21.313000 2507679 reflector.go:225] Stopping reflector *v1.PartialObjectMetadata (0s) from k8s.io/client-go/metadata/metadatainformer/informer.go:90
W1108 13:08:21.313525 2507679 mutation_detector.go:53] Mutation detector is enabled, this will result in memory leakage.
I1108 13:08:21.315741 2507679 reflector.go:219] Starting reflector *v1.PartialObjectMetadata (0s) from k8s.io/client-go/metadata/metadatainformer/informer.go:90
I1108 13:08:21.315804 2507679 reflector.go:255] Listing and watching *v1.PartialObjectMetadata from k8s.io/client-go/metadata/metadatainformer/informer.go:90
I1108 13:08:21.416699 2507679 shared_informer.go:270] caches populated
I1108 13:08:21.417285 2507679 watch.go:183] Stopping fake watcher.
I1108 13:08:21.417351 2507679 reflector.go:225] Stopping reflector *v1.PartialObjectMetadata (0s) from k8s.io/client-go/metadata/metadatainformer/informer.go:90
=== RUN   TestMetadataSharedInformerFactory/scenario_3:_test_if_deleting_an_object_triggers_DeleteFunc
W1108 13:08:21.418349 2507679 mutation_detector.go:53] Mutation detector is enabled, this will result in memory leakage.
I1108 13:08:21.420887 2507679 reflector.go:219] Starting reflector *v1.PartialObjectMetadata (0s) from k8s.io/client-go/metadata/metadatainformer/informer.go:90
I1108 13:08:21.420965 2507679 reflector.go:255] Listing and watching *v1.PartialObjectMetadata from k8s.io/client-go/metadata/metadatainformer/informer.go:90
I1108 13:08:21.521471 2507679 shared_informer.go:270] caches populated
I1108 13:08:21.521954 2507679 watch.go:183] Stopping fake watcher.
--- PASS: TestMetadataSharedInformerFactory (0.31s)
    --- PASS: TestMetadataSharedInformerFactory/scenario_1:_test_if_adding_an_object_triggers_AddFunc (0.10s)
    --- PASS: TestMetadataSharedInformerFactory/scenario_2:_tests_if_updating_an_object_triggers_UpdateFunc (0.10s)
    --- PASS: TestMetadataSharedInformerFactory/scenario_3:_test_if_deleting_an_object_triggers_DeleteFunc (0.10s)
PASS
I1108 13:08:21.522164 2507679 reflector.go:225] Stopping reflector *v1.PartialObjectMetadata (0s) from k8s.io/client-go/metadata/metadatainformer/informer.go:90
ok      k8s.io/kubernetes/vendor/k8s.io/client-go/metadata/metadatainformer     0.406s

/test pull-kubernetes-unit

[ulucinar]

We have given a try for bringing lazy OpenAPI spec marshaling changes to the active release branches release-1.22, release-1.21 and release-1.20. When opened, the v1.22 cherry-pick should look like the following:
https://github.com/kubernetes/kubernetes/compare/release-1.22...ulucinar:rel-1.22-consume-kube-openapi?expand=1

However, for the prospective v1.21 and v1.20 cherry-picks, because we are consuming kube-openapi master in the respective release branches, the "cherry-picks" involve API changes. For instance, for v1.21, we get something like the following:
https://github.com/kubernetes/kubernetes/compare/release-1.21...ulucinar:rel-1.21-consume-kube-openapi?expand=1

For release-1.21 & release-1.20 branches, we may proceed as Haowei Cai (@roycaihw) suggests in the sig-api-machinery Slack channel:

1. Fork two new branches in kube-openapi from commits consumed in the respective k/k release branches and cherry-pick the lazy marshaling PR into those branches
2. Consume these two forks in those release branches.

Do you think this is the preferred way to proceed to bring these changes into the release branches?

[muvaf]

FYI, @apelisse just created those new release branches in kube-openapi (thank you!!) and we opened three PRs [1] [2] [3] to cherry-pick the fix to those branches. Once they are merged, the cherry-pick PRs we will open here will be very limited in their scope.

[ulucinar]

The corresponding kube-openapi version bump PRs for release branches are open:
release-1.20: #106257
release-1.21: #106255
release-1.22: #106250

For v1.20 and v1.21, the PRs also bump versions for the github.com/googleapis/gnostic library.

[liggitt]

For release-1.21 & release-1.20 branches, we may proceed as Haowei Cai (@roycaihw) suggests in the sig-api-machinery Slack channel:

1. Fork two new branches in kube-openapi from commits consumed in the respective k/k release branches and cherry-pick the lazy marshaling PR into those branches
2. Consume these two forks in those release branches.

Do you think this is the preferred way to proceed to bring these changes into the release branches?

yes. we've created branches in kube-openapi before to pick targeted fixes

[fedebongio]

/triage accepted

[liggitt]

/lgtm
/approve

[liggitt]

It's pretty twisty that Get() mutates *cache, and that calls to Get() from getSwaggerBytes() and getSwaggerPbBytes() are only protected by a read-lock.

It's not clear from the outside that cache#Get() and cache#New() are unsafe to call concurrently... and the calls to Get and New only happen to be safe because they're guarded by locks (which were primarily intended to protect reads/writes to the fields in OpenAPIService).

Not blocking for this PR, but probably worth a follow-up in kube-openapi master to clarify/clean that up

[apelisse]

Good catch! Let's fix that up.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt, ulucinar

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 [liggitt]
• staging/src/k8s.io/api/OWNERS [liggitt]
• staging/src/k8s.io/legacy-cloud-providers/OWNERS [liggitt]

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