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

Always select the in-memory group/version as a target when decoding from storage #73482

Merged
merged 1 commit into from Jan 31, 2019

Conversation

@liggitt
Copy link
Member

liggitt commented Jan 29, 2019

What type of PR is this?
/kind bug

What this PR does / why we need it:
Avoids artificially limiting the source API groups we support when decoding watch events from storage. The current group version selection did not handle decoding watch events from storage resulting in writes from skewed API servers across group boundaries for cohabitating resources.

Which issue(s) this PR fixes:
Fixes #73478

Special notes for your reviewer:
We'll want to take this back to all release branches. The problem exists as far back as 1.6.

Does this PR introduce a user-facing change?:

fixes an error processing watch events when running skewed apiservers

Test output:

    --- PASS: TestCrossGroupStorage/apps/v1,_Kind=DaemonSet (0.06s)
        etcd_cross_group_test.go:141: wrote extensions/v1beta1 to etcd
        etcd_cross_group_test.go:163:      received event for apps/v1
        etcd_cross_group_test.go:163:      received event for apps/v1beta2
        etcd_cross_group_test.go:163:      received event for extensions/v1beta1
        etcd_cross_group_test.go:181:      fetched object for extensions/v1beta1
        etcd_cross_group_test.go:181:      fetched object for apps/v1
        etcd_cross_group_test.go:181:      fetched object for apps/v1beta2
        etcd_cross_group_test.go:141: wrote apps/v1 to etcd
        etcd_cross_group_test.go:163:      received event for extensions/v1beta1
        etcd_cross_group_test.go:163:      received event for apps/v1
        etcd_cross_group_test.go:163:      received event for apps/v1beta2
        etcd_cross_group_test.go:181:      fetched object for extensions/v1beta1
        etcd_cross_group_test.go:181:      fetched object for apps/v1
        etcd_cross_group_test.go:181:      fetched object for apps/v1beta2
        etcd_cross_group_test.go:141: wrote apps/v1beta2 to etcd
        etcd_cross_group_test.go:163:      received event for extensions/v1beta1
        etcd_cross_group_test.go:163:      received event for apps/v1
        etcd_cross_group_test.go:163:      received event for apps/v1beta2
        etcd_cross_group_test.go:181:      fetched object for extensions/v1beta1
        etcd_cross_group_test.go:181:      fetched object for apps/v1
        etcd_cross_group_test.go:181:      fetched object for apps/v1beta2
    --- PASS: TestCrossGroupStorage/networking.k8s.io/v1,_Kind=NetworkPolicy (0.03s)
        etcd_cross_group_test.go:141: wrote extensions/v1beta1 to etcd
        etcd_cross_group_test.go:163:      received event for extensions/v1beta1
        etcd_cross_group_test.go:163:      received event for networking.k8s.io/v1
        etcd_cross_group_test.go:181:      fetched object for extensions/v1beta1
        etcd_cross_group_test.go:181:      fetched object for networking.k8s.io/v1
        etcd_cross_group_test.go:141: wrote networking.k8s.io/v1 to etcd
        etcd_cross_group_test.go:163:      received event for extensions/v1beta1
        etcd_cross_group_test.go:163:      received event for networking.k8s.io/v1
        etcd_cross_group_test.go:181:      fetched object for extensions/v1beta1
        etcd_cross_group_test.go:181:      fetched object for networking.k8s.io/v1
    --- PASS: TestCrossGroupStorage//v1,_Kind=Event (0.04s)
        etcd_cross_group_test.go:141: wrote v1 to etcd
        etcd_cross_group_test.go:163:      received event for v1
        etcd_cross_group_test.go:163:      received event for events.k8s.io/v1beta1
        etcd_cross_group_test.go:181:      fetched object for v1
        etcd_cross_group_test.go:181:      fetched object for events.k8s.io/v1beta1
        etcd_cross_group_test.go:141: wrote events.k8s.io/v1beta1 to etcd
        etcd_cross_group_test.go:163:      received event for v1
        etcd_cross_group_test.go:163:      received event for events.k8s.io/v1beta1
        etcd_cross_group_test.go:181:      fetched object for v1
        etcd_cross_group_test.go:181:      fetched object for events.k8s.io/v1beta1
    --- PASS: TestCrossGroupStorage/apps/v1,_Kind=Deployment (0.06s)
        etcd_cross_group_test.go:141: wrote extensions/v1beta1 to etcd
        etcd_cross_group_test.go:163:      received event for apps/v1
        etcd_cross_group_test.go:163:      received event for apps/v1beta2
        etcd_cross_group_test.go:163:      received event for apps/v1beta1
        etcd_cross_group_test.go:163:      received event for extensions/v1beta1
        etcd_cross_group_test.go:181:      fetched object for apps/v1beta2
        etcd_cross_group_test.go:181:      fetched object for apps/v1beta1
        etcd_cross_group_test.go:181:      fetched object for extensions/v1beta1
        etcd_cross_group_test.go:181:      fetched object for apps/v1
        etcd_cross_group_test.go:141: wrote apps/v1 to etcd
        etcd_cross_group_test.go:163:      received event for apps/v1
        etcd_cross_group_test.go:163:      received event for apps/v1beta2
        etcd_cross_group_test.go:163:      received event for apps/v1beta1
        etcd_cross_group_test.go:163:      received event for extensions/v1beta1
        etcd_cross_group_test.go:181:      fetched object for extensions/v1beta1
        etcd_cross_group_test.go:181:      fetched object for apps/v1
        etcd_cross_group_test.go:181:      fetched object for apps/v1beta2
        etcd_cross_group_test.go:181:      fetched object for apps/v1beta1
        etcd_cross_group_test.go:141: wrote apps/v1beta2 to etcd
        etcd_cross_group_test.go:163:      received event for extensions/v1beta1
        etcd_cross_group_test.go:163:      received event for apps/v1
        etcd_cross_group_test.go:163:      received event for apps/v1beta2
        etcd_cross_group_test.go:163:      received event for apps/v1beta1
        etcd_cross_group_test.go:181:      fetched object for extensions/v1beta1
        etcd_cross_group_test.go:181:      fetched object for apps/v1
        etcd_cross_group_test.go:181:      fetched object for apps/v1beta2
        etcd_cross_group_test.go:181:      fetched object for apps/v1beta1
        etcd_cross_group_test.go:141: wrote apps/v1beta1 to etcd
        etcd_cross_group_test.go:163:      received event for extensions/v1beta1
        etcd_cross_group_test.go:163:      received event for apps/v1
        etcd_cross_group_test.go:163:      received event for apps/v1beta2
        etcd_cross_group_test.go:163:      received event for apps/v1beta1
        etcd_cross_group_test.go:181:      fetched object for apps/v1
        etcd_cross_group_test.go:181:      fetched object for apps/v1beta2
        etcd_cross_group_test.go:181:      fetched object for apps/v1beta1
        etcd_cross_group_test.go:181:      fetched object for extensions/v1beta1
    --- PASS: TestCrossGroupStorage/apps/v1,_Kind=ReplicaSet (0.05s)
        etcd_cross_group_test.go:141: wrote extensions/v1beta1 to etcd
        etcd_cross_group_test.go:163:      received event for extensions/v1beta1
        etcd_cross_group_test.go:163:      received event for apps/v1
        etcd_cross_group_test.go:163:      received event for apps/v1beta2
        etcd_cross_group_test.go:181:      fetched object for extensions/v1beta1
        etcd_cross_group_test.go:181:      fetched object for apps/v1
        etcd_cross_group_test.go:181:      fetched object for apps/v1beta2
        etcd_cross_group_test.go:141: wrote apps/v1 to etcd
        etcd_cross_group_test.go:163:      received event for apps/v1
        etcd_cross_group_test.go:163:      received event for apps/v1beta2
        etcd_cross_group_test.go:163:      received event for extensions/v1beta1
        etcd_cross_group_test.go:181:      fetched object for extensions/v1beta1
        etcd_cross_group_test.go:181:      fetched object for apps/v1
        etcd_cross_group_test.go:181:      fetched object for apps/v1beta2
        etcd_cross_group_test.go:141: wrote apps/v1beta2 to etcd
        etcd_cross_group_test.go:163:      received event for extensions/v1beta1
        etcd_cross_group_test.go:163:      received event for apps/v1
        etcd_cross_group_test.go:163:      received event for apps/v1beta2
        etcd_cross_group_test.go:181:      fetched object for extensions/v1beta1
        etcd_cross_group_test.go:181:      fetched object for apps/v1
        etcd_cross_group_test.go:181:      fetched object for apps/v1beta2
    --- PASS: TestCrossGroupStorage/policy/v1beta1,_Kind=PodSecurityPolicy (0.03s)
        etcd_cross_group_test.go:141: wrote extensions/v1beta1 to etcd
        etcd_cross_group_test.go:163:      received event for extensions/v1beta1
        etcd_cross_group_test.go:163:      received event for policy/v1beta1
        etcd_cross_group_test.go:181:      fetched object for extensions/v1beta1
        etcd_cross_group_test.go:181:      fetched object for policy/v1beta1
        etcd_cross_group_test.go:141: wrote policy/v1beta1 to etcd
        etcd_cross_group_test.go:163:      received event for extensions/v1beta1
        etcd_cross_group_test.go:163:      received event for policy/v1beta1
        etcd_cross_group_test.go:181:      fetched object for extensions/v1beta1
        etcd_cross_group_test.go:181:      fetched object for policy/v1beta1
@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Jan 29, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt

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

@liggitt liggitt changed the title WIP - Always select the in-memory group/version as a target when decoding from storage Always select the in-memory group/version as a target when decoding from storage Jan 29, 2019

@liggitt

This comment has been minimized.

Copy link
Member Author

liggitt commented Jan 29, 2019

added a test to ensure all cross-group resources are watchable and fetchable from all exposed group/versions, no matter which version gets persisted in etcd

@liggitt

This comment has been minimized.

Copy link
Member Author

liggitt commented Jan 29, 2019

/priority critical-urgent

@caesarxuchao

This comment has been minimized.

Copy link
Member

caesarxuchao commented Jan 29, 2019

/assign

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Jan 29, 2019

@liggitt: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-e2e-kops-aws 90e3327 link /test pull-kubernetes-e2e-kops-aws

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.

@caesarxuchao
Copy link
Member

caesarxuchao left a comment

I can think of a more targeted fix, but I think it isn't worth the complexity.

Some minor comments.

@liggitt

This comment has been minimized.

Copy link
Member Author

liggitt commented Jan 30, 2019

/skip

@liggitt liggitt force-pushed the liggitt:cross-group-watch branch from 90e3327 to 131dad6 Jan 30, 2019

@caesarxuchao

This comment has been minimized.

Copy link
Member

caesarxuchao commented Jan 31, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label Jan 31, 2019

@k8s-ci-robot k8s-ci-robot merged commit fb96afb into kubernetes:master Jan 31, 2019

16 checks passed

cla/linuxfoundation liggitt authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-cross Skipped
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-godeps Job succeeded.
Details
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped
pull-kubernetes-local-e2e-containerized Skipped
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
tide In merge pool.
Details

@liggitt liggitt deleted the liggitt:cross-group-watch branch Jan 31, 2019

k8s-ci-robot added a commit that referenced this pull request Feb 8, 2019

Merge pull request #73564 from liggitt/automated-cherry-pick-of-#7348…
…2-upstream-release-1.11

Automated cherry pick of #73482: Always select the in-memory group/version as a target when

k8s-ci-robot added a commit that referenced this pull request Feb 11, 2019

Merge pull request #73562 from liggitt/automated-cherry-pick-of-#7348…
…2-upstream-release-1.13

Automated cherry pick of #73482: Always select the in-memory group/version as a target when

k8s-ci-robot added a commit that referenced this pull request Feb 11, 2019

Merge pull request #73563 from liggitt/automated-cherry-pick-of-#7348…
…2-upstream-release-1.12

Automated cherry pick of #73482: Always select the in-memory group/version as a target when
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment