Skip to content

feat: add v2 pod controller healthstatus and proxyconfiguration#2911

Merged
DanStough merged 1 commit intomainfrom
dans/NET-5315/v2-pods-controller-health-n-proxycfg
Sep 11, 2023
Merged

feat: add v2 pod controller healthstatus and proxyconfiguration#2911
DanStough merged 1 commit intomainfrom
dans/NET-5315/v2-pods-controller-health-n-proxycfg

Conversation

@DanStough
Copy link
Copy Markdown
Contributor

@DanStough DanStough commented Sep 6, 2023 •
edited
Loading

Changes proposed in this PR:

  • Adds support in the V2 Pod Controller for HealthStatuses and ProxyConfigurations.
  • Enterprise tests and logging will come in a subsequent PR.

How I've tested this PR:

  • Unit and integration tests as included in the 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 6, 2023
@DanStough DanStough force-pushed the dans/NET-5315/v2-pods-controller-health-n-proxycfg branch from 69630ff to 616fd9c Compare September 7, 2023 15:51
zalimeni
zalimeni approved these changes Sep 7, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@zalimeni zalimeni left a comment •
edited
Loading

Choose a reason for hiding this comment

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

A few minor suggestions and one more significant question about cascading deletes - otherwise LGTM!

Approving to unblock based on whether you think the comments are actionable; if so happy to re-review as needed.

@DanStough DanStough requested a review from zalimeni September 8, 2023 17:13
@DanStough
Copy link
Copy Markdown
Contributor Author

DanStough commented Sep 8, 2023

@zalimeni maybe you can just take a peek again at the bit where I add the PeerName and make sure it makes sense to you?

zalimeni
zalimeni approved these changes Sep 8, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@zalimeni zalimeni left a comment

Choose a reason for hiding this comment

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

LGTM re: PeerName addition, though wouldn’t hurt to add a comment explaining why it’s (temporarily) needed. I feel like the follow up volume just keeps growing and won’t age well… 😬

@DanStough DanStough force-pushed the dans/NET-5315/v2-pods-controller-health-n-proxycfg branch from 2c3d411 to cdcf931 Compare September 8, 2023 20:04
@DanStough DanStough force-pushed the dans/NET-5315/v2-pods-controller-health-n-proxycfg branch from cdcf931 to 51debbe Compare September 11, 2023 14:11
@DanStough DanStough merged commit 6e8831f into main Sep 11, 2023
@DanStough DanStough deleted the dans/NET-5315/v2-pods-controller-health-n-proxycfg branch September 11, 2023 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ndhanushkodi ndhanushkodi Awaiting requested review from ndhanushkodi

@thisisnotashwin thisisnotashwin Awaiting requested review from thisisnotashwin

1 more reviewer

@zalimeni zalimeni zalimeni approved these changes

Reviewers whose approvals may not affect merge requirements

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.

2 participants

@DanStough @zalimeni