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

hooks: split feature detection from setup and validation #2270

Merged
merged 1 commit into from
Jan 30, 2024

Conversation

giorio94
Copy link
Member

Currently, the SetupAndValidate extension hook is executed right after feature detection, but before the setup and validation of the different connectivity test pods and artifacts. While this has the advantage of nicely integrating with the logging of the detected features, it does not allow the hook function to rely on the existence of the pods created immediately afterwards. To enable both functionalities, let's split the hook into two -- one first function to detect extra features, and a second to perform the actual setup and validation of extra pods.

Copy link
Contributor

@derailed derailed left a comment

Choose a reason for hiding this comment

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

@giorio94 Thanks for the update Marco!
Perhaps leveraging interface composition (if possible) could dry this up a bit more?

connectivity/suite.go Outdated Show resolved Hide resolved
internal/cli/cmd/hooks.go Outdated Show resolved Hide resolved
Currently, the SetupAndValidate extension hook is executed right after
feature detection, but before the setup and validation of the different
connectivity test pods and artifacts. While this has the advantage of
nicely integrating with the logging of the detected features, it does
not allow the hook function to rely on the existence of the pods created
immediately afterwards. To enable both functionalities, let's split the
hook into two -- one first function to detect extra features, and a second
to perform the actual setup and validation of extra pods.

Signed-off-by: Marco Iorio <[email protected]>
@giorio94
Copy link
Member Author

Rebased to pick the fix for the EKS tunnel workflow

Copy link
Contributor

@derailed derailed left a comment

Choose a reason for hiding this comment

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

@giorio94 LGTM

@michi-covalent michi-covalent merged commit a3b50b4 into cilium:main Jan 30, 2024
13 checks passed
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