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

feat(control-plane): v2 backoff on missing namespace #2960

Merged
merged 1 commit into from
Sep 18, 2023
Merged

feat(control-plane): v2 backoff on missing namespace #2960

merged 1 commit into from
Sep 18, 2023

Conversation

DanStough
Copy link
Contributor

Changes proposed in this PR:

  • Add requeue logic when the namespace is missing, which could be the case before the namespace controller has reconciled it.

How I've tested this PR: Unit tests on the helper func, but not the controller. We have a task to come back to the enterprise tests. I'm also fine lumping that into this PR.

How I expect reviewers to test this PR: 👀

Checklist:

@DanStough DanStough added pr/no-changelog PR does not need a corresponding .changelog entry pr/no-backport signals that a PR will not contain a backport label labels Sep 14, 2023
@DanStough DanStough force-pushed the dans/backoff-missing-v2-resources branch from 670f20c to fdb4841 Compare September 14, 2023 15:27
@DanStough DanStough force-pushed the dans/backoff-missing-v2-resources branch from fdb4841 to 529ddf4 Compare September 14, 2023 15:37
wilkermichael
wilkermichael approved these changes Sep 14, 2023
View reviewed changes
Copy link
Contributor

@wilkermichael wilkermichael left a comment

Choose a reason for hiding this comment

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

Looks great Dan!

Love all the retries 😍

Just some minor comments/questions

control-plane/connect-inject/common/common.go Outdated Show resolved Hide resolved
control-plane/connect-inject/common/common.go Show resolved Hide resolved
control-plane/connect-inject/common/common_test.go Show resolved Hide resolved
control-plane/connect-inject/controllers/pod/pod_controller.go Outdated Show resolved Hide resolved
@DanStough DanStough force-pushed the dans/backoff-missing-v2-resources branch from d41a28e to feb0349 Compare September 15, 2023 03:39
@DanStough DanStough force-pushed the dans/backoff-missing-v2-resources branch from feb0349 to de73cec Compare September 15, 2023 21:29
@DanStough DanStough enabled auto-merge (squash) September 15, 2023 21:36
@DanStough DanStough force-pushed the dans/backoff-missing-v2-resources branch from de73cec to 4785ccc Compare September 18, 2023 21:56
@DanStough DanStough merged commit a4616cf into main Sep 18, 2023
22 of 31 checks passed
@DanStough DanStough deleted the dans/backoff-missing-v2-resources branch September 18, 2023 22:25
@zalimeni zalimeni mentioned this pull request Sep 19, 2023
Merged
2 tasks
@zalimeni zalimeni mentioned this pull request Oct 3, 2023
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zalimeni zalimeni zalimeni approved these changes

@thisisnotashwin thisisnotashwin thisisnotashwin approved these changes

@wilkermichael wilkermichael wilkermichael approved these changes

Assignees
No one assigned
Labels
pr/no-backport signals that a PR will not contain a backport label pr/no-changelog PR does not need a corresponding .changelog entry
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@DanStough @zalimeni @thisisnotashwin @wilkermichael