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

[3.5] backport ignore old leaders leases revoking requests #17425

Merged
merged 2 commits into from
Feb 18, 2024
Merged

[3.5] backport ignore old leaders leases revoking requests #17425

merged 2 commits into from
Feb 18, 2024

Conversation

ivanvc
Copy link
Member

@ivanvc ivanvc commented Feb 14, 2024 •
edited
Loading

Backport of PR #16822.

This PR has two commits:
1. Backport e2e EtcdProecess' GoFailClientTimeout. Addressed in #17427
2. Backport of the actual implementation of ignoring old leader leases revoking requests.

Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.

@k8s-ci-robot
Copy link

Hi @ivanvc. Thanks for your PR.

I'm waiting for a etcd-io 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.

@ahrtr
Copy link
Member

ahrtr commented Feb 14, 2024

Can you backport the change of supporting/enhancing gofailpoint in a separate PR?

@ivanvc ivanvc mentioned this pull request Feb 14, 2024
Merged
@ahrtr
Copy link
Member

ahrtr commented Feb 15, 2024

#17427 has merged, please rebase this PR

@ivanvc ivanvc force-pushed the release-3.5-backport-ignore-old-leaders-leases-revoking-request branch from b191670 to 89bcfc2 Compare February 15, 2024 18:03
@ivanvc
Copy link
Member Author

ivanvc commented Feb 15, 2024

#17427 has merged, please rebase this PR

Done :)

ahrtr
ahrtr approved these changes Feb 15, 2024
View reviewed changes
Copy link
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

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

lgtm

Thanks.

@ahrtr
Copy link
Member

ahrtr commented Feb 16, 2024

cc @serathius @fuweid @jmhbnz

jmhbnz
jmhbnz approved these changes Feb 16, 2024
View reviewed changes
Copy link
Member

@jmhbnz jmhbnz left a comment

Choose a reason for hiding this comment

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

Main backport looks good to me, thanks @ivanvc.

One question below about EndpointsGRPC inclusion and how this is treated.

tests/framework/e2e/cluster.go Show resolved Hide resolved
@ivanvc ivanvc force-pushed the release-3.5-backport-ignore-old-leaders-leases-revoking-request branch from 89bcfc2 to da4816d Compare February 17, 2024 04:19
jmhbnz
jmhbnz approved these changes Feb 17, 2024
View reviewed changes
Copy link
Member

@jmhbnz jmhbnz left a comment

Choose a reason for hiding this comment

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

LGTM on green - Thanks @ivanvc

@ahrtr
Copy link
Member

ahrtr commented Feb 17, 2024

ping @fuweid @serathius

fuweid
fuweid approved these changes Feb 17, 2024
View reviewed changes
Copy link
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM

I think we can use git cherry-pick -xs to add original commit information so that it's easy to review and track when you read commits.

@ivanvc
Copy link
Member Author

ivanvc commented Feb 18, 2024

LGTM

I think we can use git cherry-pick -xs to add original commit information so that it's easy to review and track when you read commits.

Hi @fuweid, I can't cherry-pick cleanly when backporting as there are many differences between the main branch and release-3.{4,5}, it's usually something done by hand. But I agree that adding a reference to the original commit can be useful. I'll add a reference to the original backported commit in the commit message.

@ivanvc ivanvc force-pushed the release-3.5-backport-ignore-old-leaders-leases-revoking-request branch 2 times, most recently from 4e530ef to bd6dfdc Compare February 18, 2024 06:13
@ivanvc
tests/e2e: backport e2e cluster setup
c486b58 
Finish backporting the remaining functions from the original backport
from PR etcd-io#17075.

Backported commit 65add8c

Signed-off-by: Ivan Valdes <[email protected]>
@ivanvc
Backport ignore old leader's leases revoking request
4a90575 
Backported PR etcd-io#16822, commits f7e488d,
67f1716,
and f7ff898

Signed-off-by: Ivan Valdes <[email protected]>
@ivanvc ivanvc force-pushed the release-3.5-backport-ignore-old-leaders-leases-revoking-request branch from bd6dfdc to 4a90575 Compare February 18, 2024 06:20
@ivanvc
Copy link
Member Author

ivanvc commented Feb 18, 2024

@ahrtr, sorry for the inconvenience, but I've rebased and edited the commits so many times that I realized in one pass I forgot to add the e2e tests. Can you please review it again? Thanks.

@ivanvc ivanvc requested a review from ahrtr February 18, 2024 06:23
@ahrtr ahrtr merged commit 9ffba74 into etcd-io:release-3.5 Feb 18, 2024
15 checks passed
@ahrtr
Copy link
Member

ahrtr commented Feb 18, 2024

Link to #15247

@ivanvc ivanvc deleted the release-3.5-backport-ignore-old-leaders-leases-revoking-request branch February 18, 2024 15:19
dusk125 added a commit to dusk125/etcd that referenced this pull request Jul 22, 2024
@dusk125
Revert "Merge pull request etcd-io#17425 from ivanvc/release-3.5-back…
a988d5e 
…port-ignore-old-leaders-leases-revoking-request"

This reverts commit 9ffba74, reversing
changes made to c213177.
dusk125 added a commit to dusk125/etcd that referenced this pull request Jul 22, 2024
@dusk125
Revert "Merge pull request etcd-io#17425 from ivanvc/release-3.5-back…
0832956 
…port-ignore-old-leaders-leases-revoking-request"

This reverts commit 9ffba74, reversing
changes made to c213177.
dusk125 added a commit to dusk125/etcd that referenced this pull request Jul 22, 2024
@dusk125
Revert "Merge pull request etcd-io#17425 from ivanvc/release-3.5-back…
0340c60 
…port-ignore-old-leaders-leases-revoking-request"

This reverts commit 9ffba74, reversing
changes made to c213177.
@ahrtr ahrtr mentioned this pull request Aug 8, 2024
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmhbnz jmhbnz jmhbnz approved these changes

@ahrtr ahrtr ahrtr approved these changes

@fuweid fuweid fuweid approved these changes

@serathius serathius serathius approved these changes

Assignees
No one assigned
Labels
ok-to-test
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ivanvc @k8s-ci-robot @ahrtr @serathius @fuweid @jmhbnz