Skip to content

refactor: address comments from the previous VAP PR#1181

Merged
nwnt merged 6 commits into
Azure:mainfrom
nwnt:address-vap-comments
Sep 16, 2025
Merged

refactor: address comments from the previous VAP PR#1181
nwnt merged 6 commits into
Azure:mainfrom
nwnt:address-vap-comments

Conversation

@nwnt

@nwnt nwnt commented Aug 26, 2025

Copy link
Copy Markdown
Contributor

Description of your changes

Addresses the comments from Ryan in #1179 1179

I have:

  • Run make reviewable to ensure this PR is ready for review.

How has this code been tested

Running e2e tests successfully.

Special notes for your reviewer

@Arvindthiru

Arvindthiru commented Sep 3, 2025

Copy link
Copy Markdown
Contributor

Can we also address,

Comment thread test/e2e/managed_resource_vap_test.go
Comment thread test/e2e/managed_resource_vap_test.go Outdated
Comment thread test/e2e/managed_resource_vap_test.go Outdated
Comment thread test/e2e/managed_resource_vap_test.go Outdated
Comment thread test/e2e/managed_resource_vap_test.go Outdated
Comment thread test/e2e/managed_resource_vap_test.go Outdated
@nwnt
nwnt force-pushed the address-vap-comments branch 2 times, most recently from b908570 to 452a860 Compare September 10, 2025 22:36
Comment thread test/e2e/managed_resource_vap_test.go
Comment thread test/e2e/managed_resource_vap_test.go Outdated
Nont added 3 commits September 11, 2025 15:51
Signed-off-by: Nont <nont@duck.com>
Signed-off-by: Nont <nont@duck.com>
Signed-off-by: Nont <nont@duck.com>
@nwnt
nwnt force-pushed the address-vap-comments branch from 452a860 to 54511c6 Compare September 11, 2025 20:58
Signed-off-by: Nont <nont@duck.com>
@nwnt
nwnt force-pushed the address-vap-comments branch from 54511c6 to a54993b Compare September 12, 2025 19:59
Comment thread test/e2e/setup_test.go
@nwnt
nwnt force-pushed the address-vap-comments branch from 7e5c95e to a54993b Compare September 12, 2025 20:57
@Arvindthiru

Copy link
Copy Markdown
Contributor

Can we also fix this existing E2E should use systemMasterUser for update https://github.com/Azure/fleet/blob/main/test/e2e/managed_resource_vap_test.go#L168

Comment thread test/e2e/managed_resource_vap_test.go Outdated
Signed-off-by: Nont <nont@duck.com>
britaniar
britaniar previously approved these changes Sep 15, 2025
Comment thread test/e2e/managed_resource_vap_test.go Outdated

@Arvindthiru Arvindthiru left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could we also cover an update case in the e2e for a resource from placement.kubernetes-fleet.io ?

Signed-off-by: Nont <nont@duck.com>
@nwnt
nwnt merged commit 5158d08 into Azure:main Sep 16, 2025
25 of 26 checks passed
@nwnt
nwnt deleted the address-vap-comments branch September 16, 2025 01:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants