Skip to content
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

Automated cherry pick of #98116: slice mirroring controller mirror annotations #100443

Merged
merged 1 commit into from Mar 29, 2021
Merged

Automated cherry pick of #98116: slice mirroring controller mirror annotations #100443

merged 1 commit into from Mar 29, 2021

Conversation

aojea
Copy link
Member

@aojea aojea commented Mar 22, 2021 •
edited

Cherry pick of #98116 on release-1.20.

#98116: slice mirroring controller mirror annotations

For details on the cherry pick process, see the cherry pick requests page.

Fixes: #98577

The endpointslice mirroring controller mirrors endpoints annotations and labels to the generated endpoint slices, it also ensures that updates on any of these fields on the endpoints are mirrored. 
The well-known annotation endpoints.kubernetes.io/last-change-trigger-time is skipped and not mirrored.

@k8s-ci-robot k8s-ci-robot added this to the v1.20 milestone Mar 22, 2021
@k8s-ci-robot k8s-ci-robot added do-not-merge/cherry-pick-not-approved Indicates that a PR is not yet approved to merge into a release branch. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. 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. area/test labels Mar 22, 2021
@k8s-ci-robot k8s-ci-robot added sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/network Categorizes an issue or PR as relevant to SIG Network. sig/testing Categorizes an issue or PR as relevant to SIG Testing. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Mar 22, 2021
@aojea aojea force-pushed the automated-cherry-pick-of-#98116-upstream-release-1.20 branch from 5953a07 to a5ed0ba Compare March 22, 2021 07:56
@aojea
Copy link
Member Author

aojea commented Mar 22, 2021

/kind bug
/sig network
/assign @robscott
/cc @maplain

@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Mar 22, 2021
@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Mar 22, 2021
@aojea aojea force-pushed the automated-cherry-pick-of-#98116-upstream-release-1.20 branch from a5ed0ba to 7cf02df Compare March 22, 2021 08:08
@aojea
slice mirroring controller mirror annotations
a53e27f 
Add support to the endpoint slice mirroring controller to mirror
annotations, in addition to labels, but don´t mirror endpoint
triggertime annotation.

Also, fix a bug in the endpointslice mirroring controller, that
wasn't updating the mirrored slice with the new labels, in case
that only the endpoint labels were modified.
@aojea aojea force-pushed the automated-cherry-pick-of-#98116-upstream-release-1.20 branch from 7cf02df to a53e27f Compare March 22, 2021 09:56
@aojea
Copy link
Member Author

aojea commented Mar 22, 2021

/retest

1 similar comment
@aojea
Copy link
Member Author

aojea commented Mar 23, 2021

/retest

@robscott
Copy link
Member

Thanks!

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 23, 2021
@robscott
Copy link
Member

@MrHohn do you mind taking a look at this one? You approved the /test changes in the original PR.

/assign @MrHohn

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 23, 2021
@robscott
Copy link
Member

@aojea Yep, they merged earlier in #100113 and #100143. Should probably also consider a pick to 1.19 for this.

@aojea
Copy link
Member Author

aojea commented Mar 23, 2021

/assign @puerco @cpanato

@aojea aojea mentioned this pull request Mar 23, 2021
Merged
@cpanato
Copy link
Member

cpanato commented Mar 24, 2021

@aojea we need this just for 1.20 branch?

@aojea
Copy link
Member Author

aojea commented Mar 24, 2021

@aojea we need this just for 1.20 branch?

I did it for 1.19 too #100504

Just 1.20 and 1.19

@cpanato
Copy link
Member

cpanato commented Mar 24, 2021

@aojea the PR for the main branch says it is both kind/bug/kind/feature just would like to have more clarification on this if possible before moving forward.
This is a bug fix with a feature being added as well?
thanks!

@aojea
Copy link
Member Author

aojea commented Mar 24, 2021

@aojea the PR for the main branch says it is both kind/bug/kind/feature just would like to have more clarification on this if possible before moving forward.
This is a bug fix with a feature being added as well?
thanks!

ahh right, great catch.
I remember now, so it boils down to consider if the endpoinslices mirroring should mirror annotations since the first day or no, if you see the open issue #98026, it was not clear at that time, so I carried over with that assumption.

We need to ask @robscott if mirroring annotations is a new feature or a bug, and I will update the PRs accordingly

@cpanato
Copy link
Member

cpanato commented Mar 24, 2021

thank you so much for your clarification @aojea , lets wait for Rob

@aojea
Copy link
Member Author

aojea commented Mar 24, 2021

thank you so much for your clarification @aojea , lets wait for Rob

I personally think in hindsight, based on the additional bug reported here #98577, that this is a bugfix, just that we missed to consider annotations to be mirrored at design time ... but Rob is the judge here

@cpanato
Copy link
Member

cpanato commented Mar 24, 2021

IMHO is a bug fix to me as weel, just would like the clarification because in the main PR there is a kind/feature in the labels
sorry about being picky :)

@robscott
Copy link
Member

Agree that this is a bug fix. This could be categorized as a feature since it's adding functionality that didn't already exist, but I think it's a bug because this functionality should have existed from the beginning and the lack of it could break things.

@cpanato
Copy link
Member

cpanato commented Mar 29, 2021

/triage accepted

from parent PR
/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 Mar 29, 2021
cpanato
cpanato approved these changes Mar 29, 2021
View reviewed changes
Copy link
Member

@cpanato cpanato left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aojea, cpanato, MrHohn, robscott

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

@cpanato cpanato added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Mar 29, 2021
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/cherry-pick-not-approved Indicates that a PR is not yet approved to merge into a release branch. label Mar 29, 2021
@cpanato
Copy link
Member

cpanato commented Mar 29, 2021

/test pull-kubernetes-e2e-kind

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

1 similar comment
@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

k8s-ci-robot added a commit that referenced this pull request Mar 29, 2021
@k8s-ci-robot
Merge pull request #100504 from aojea/automated-cherry-pick-of-#10044…
1a11e2a 
…3-upstream-release-1.19

Automated cherry pick of #100443: slice mirroring controller mirror annotations
@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@k8s-ci-robot k8s-ci-robot merged commit 7849986 into kubernetes:release-1.20 Mar 29, 2021
@liggitt liggitt mentioned this pull request May 6, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cpanato cpanato cpanato approved these changes

@MrHohn MrHohn MrHohn approved these changes

@danwinship danwinship Awaiting requested review from danwinship

@johnSchnake johnSchnake Awaiting requested review from johnSchnake

@maplain maplain Awaiting requested review from maplain

Assignees

@robscott robscott

@puerco puerco

@cpanato cpanato

@MrHohn MrHohn

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/test cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. 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 Denotes a PR that will be considered when it comes time to generate release notes. sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/network Categorizes an issue or PR as relevant to SIG Network. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Milestone
v1.20
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@aojea @robscott @cpanato @k8s-ci-robot @fejta-bot @MrHohn @puerco