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.

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

Clean up stale resources when 'antctl check cluster' failed #6597

Merged
merged 1 commit into from
Aug 21, 2024

Conversation

luolanzone
Copy link
Contributor

No description provided.

@luolanzone luolanzone requested review from antoninbas and tnqn August 8, 2024 03:48
@luolanzone luolanzone force-pushed the antctl-cleanup-temp-namespace branch from 5beaa37 to e542c29 Compare August 8, 2024 07:02
@luolanzone luolanzone marked this pull request as draft August 8, 2024 07:51
@luolanzone
Copy link
Contributor Author

Checking on the antctl check installation e2e failure, convert this to a draft one.

@luolanzone luolanzone force-pushed the antctl-cleanup-temp-namespace branch 2 times, most recently from aac6af7 to fc915cd Compare August 9, 2024 08:17
@luolanzone luolanzone marked this pull request as ready for review August 9, 2024 08:18
@@ -84,6 +88,7 @@ func (t *DenyAllConnectivityTest) Run(ctx context.Context, testContext *testCont
testContext.Log("NetworkPolicy deletion was successful")
return nil
}()
time.Sleep(networkPolicyDelay)
Copy link
Contributor Author

@luolanzone luolanzone Aug 9, 2024

Choose a reason for hiding this comment

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

The *-deny-all e2e failures should have nothing to do with the changes for the stale resources cleanup, it's likely to be an issue with NP reconciliation which caused the service is connectable when deny is expected. So I added 2 seconds here to wait NP to be reconciled before the testing.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer something poll-based, but I guess this can be addressed in a later PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I can check later for poll-based with a new PR. Thanks.

pkg/antctl/raw/check/cluster/command.go Outdated Show resolved Hide resolved
@@ -84,6 +88,7 @@ func (t *DenyAllConnectivityTest) Run(ctx context.Context, testContext *testCont
testContext.Log("NetworkPolicy deletion was successful")
return nil
}()
time.Sleep(networkPolicyDelay)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer something poll-based, but I guess this can be addressed in a later PR

@luolanzone luolanzone force-pushed the antctl-cleanup-temp-namespace branch from fc915cd to e0cf413 Compare August 19, 2024 02:13
Signed-off-by: Lan Luo <luola@vmware.com>
@luolanzone luolanzone force-pushed the antctl-cleanup-temp-namespace branch from e0cf413 to 4787f28 Compare August 20, 2024 03:35
@antoninbas
Copy link
Contributor

/skip-all

Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

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

LGTM

@tnqn tnqn merged commit dc9152d into antrea-io:main Aug 21, 2024
52 of 58 checks passed
@luolanzone luolanzone deleted the antctl-cleanup-temp-namespace branch October 23, 2024 02:47
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