Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Merged
k8s-ci-robot merged 1 commit into from
Jan 31, 2019
Merged

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

k8s-ci-robot merged 1 commit into from
Jan 31, 2019
+286 −1

Conversation

@liggitt
Copy link
Member

@liggitt liggitt commented Jan 29, 2019 •
edited
Loading

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 k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/bug Categorizes issue or PR as related to a bug. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. area/apiserver sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jan 29, 2019
@k8s-ci-robot k8s-ci-robot added 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 Jan 29, 2019
@k8s-ci-robot
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

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 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
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 29, 2019
@liggitt
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
Copy link
Member Author

liggitt commented Jan 29, 2019

/priority critical-urgent

@k8s-ci-robot k8s-ci-robot added priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jan 29, 2019
@caesarxuchao
Copy link
Contributor

caesarxuchao commented Jan 29, 2019

/assign

@k8s-ci-robot
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 90e3327148accaec1080b9ea0909b80953e6bf68 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.

Details

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
caesarxuchao reviewed Jan 29, 2019
View reviewed changes
Copy link
Contributor

@caesarxuchao caesarxuchao left a comment

Choose a reason for hiding this comment

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

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

Some minor comments.

@liggitt
Copy link
Member Author

liggitt commented Jan 30, 2019

/skip

@caesarxuchao
Copy link
Contributor

caesarxuchao commented Jan 31, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 31, 2019
@k8s-ci-robot k8s-ci-robot merged commit fb96afb into kubernetes:master Jan 31, 2019
This was referenced Jan 31, 2019
Merged
Merged
Merged
@liggitt liggitt deleted the cross-group-watch branch January 31, 2019 15:12
k8s-ci-robot added a commit that referenced this pull request Feb 8, 2019
@k8s-ci-robot
Merge pull request #73564 from liggitt/automated-cherry-pick-of-#7348…
6ab580c 
…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
@k8s-ci-robot
Merge pull request #73562 from liggitt/automated-cherry-pick-of-#7348…
b5bf78c 
…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
@k8s-ci-robot
Merge pull request #73563 from liggitt/automated-cherry-pick-of-#7348…
59d6f96 
…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

Reviewers

@sttts sttts Awaiting requested review from sttts

1 more reviewer

@caesarxuchao caesarxuchao caesarxuchao left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@caesarxuchao caesarxuchao

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/apiserver 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/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/testing Categorizes an issue or PR as relevant to SIG Testing.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Cross-group watch events from etcd are not handled correctly

3 participants

@liggitt @k8s-ci-robot @caesarxuchao