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

add setup hook capabilities for rke2 #2146

Merged
merged 3 commits into from
Aug 20, 2020

Conversation

briandowns
Copy link
Contributor

Proposed changes

This PR adds the ability to add hooks to be run in bootstrapping in k3s that will ultimately be used during rke2 startup.

Types of changes

Adds a slice to a struct field and referenced during k3s start up.

Verification

Verification will be had through it's use in this RKE2 pull request. rancher/rke2#199

Linked Issues

N/A

Further comments

N/A

@briandowns briandowns requested a review from a team August 19, 2020 20:36
@briandowns briandowns self-assigned this Aug 19, 2020
Copy link
Member

@brandond brandond left a comment

Choose a reason for hiding this comment

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

Might these better be called StartupHooks? They're called in StartServer, after the apiserver and controllers are started, well after any other setup has already been done.

Based on the current name I was expecting to be able to do some setup, maybe alter the configuration if I needed to customize something before it was used to set up the rest of the system.

@briandowns
Copy link
Contributor Author

They'd be used during RKE2 setup. I open to calling them anything that makes their use clear.

@brandond
Copy link
Member

So k3s calls these hooks during startup, but rke2 calls them during setup? Or does rke2 add them during setup? I'm in favor of naming them after the place they get called.

Signed-off-by: Brian Downs <[email protected]>
@briandowns
Copy link
Contributor Author

changed

Copy link
Member

@brandond brandond left a comment

Choose a reason for hiding this comment

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

❤️ 🇮🇹

pkg/server/server.go Outdated Show resolved Hide resolved
@briandowns
Copy link
Contributor Author

Updated. Thanks @cjellick @dweomer @brandond

Copy link
Member

@brandond brandond left a comment

Choose a reason for hiding this comment

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

Good call on the context and error handling.

@briandowns briandowns merged commit a2471a1 into k3s-io:master Aug 20, 2020
@cjellick cjellick mentioned this pull request Sep 3, 2020
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