Rework OSM integration into generic external bootstrap "interface" #1428
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kubermatic-bot
added
release-note
|
Skipping CI for Draft Pull Request. |
kubermatic-bot
added
sig/cluster-management
Signed-off-by: Marvin Beckers <[email protected]>
Signed-off-by: Marvin Beckers <[email protected]>
Signed-off-by: Marvin Beckers <[email protected]>
embik
force-pushed
the
external-bootstrap-mechanism
branch
from
29eb2e8 to
18ee0ff
Compare
…isabled Signed-off-by: Marvin Beckers <[email protected]>
embik
changed the title
kubermatic-bot
removed
the
do-not-merge/work-in-progress
Signed-off-by: Marvin Beckers <[email protected]>
|
/retest |
embik
changed the title
kubermatic-bot
added
the
do-not-merge/work-in-progress
Signed-off-by: Marvin Beckers <[email protected]>
|
/retest |
embik
changed the title
kubermatic-bot
removed
the
do-not-merge/work-in-progress
|
/retest |
ahmedwaleedmalik
approved these changes
Oct 4, 2022
There was a problem hiding this comment.
Great job @embik, this brings us a lot closer to what we wanted to achieve; remove this circle of dependency between OSM and MC.
/approve 💙
|
LGTM label has been added. Git tree hash: ba643ae37221b57b859e0c2207a568ba3d48b70e
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ahmedwaleedmalik, embik The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
kubermatic-bot
added
the
approved
|
/retest |
mate4st
pushed a commit
to mate4st/machine-controller
that referenced
this pull request
Mar 13, 2023
…ubermatic#1428) * Deprecate use-osm flag, remove osm dependency Signed-off-by: Marvin Beckers <[email protected]> * Add a note to pkg/bootstrap/types.go Signed-off-by: Marvin Beckers <[email protected]> * Remove provisioning secret as implementation detail of OSM Signed-off-by: Marvin Beckers <[email protected]> * Add legacy label again but also set it if external bootstrapping is disabled Signed-off-by: Marvin Beckers <[email protected]> * Fix linting issues Signed-off-by: Marvin Beckers <[email protected]> * Annotate testdata with default OSPs Signed-off-by: Marvin Beckers <[email protected]> Signed-off-by: Marvin Beckers <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
This PR is essentially both proposal and code change in one go. Let me explain the reasoning behind this PR first:
Essentially, we have a consumer/producer relationship between MC (as consumer) and OSM (as producer) when it comes to the user-data passed to new machines. However, I believe the consumer/producer relationship bits are a bit all over the place. Some logic exists in MC, some in OSM. The consumer/producer relationship here is very similar to Go interfaces, and a rule of thumb there is that interfaces should not be aware of types that implement them.
The same, IMHO, goes for the MC-OSM relationship: MC should support a standard (or interface, if you want;
pkg/bootstrapdefines this in this PR) that any producer of bootstrapping user-data can "slot" into. But MC should not be aware of the producer component in any way, other than the defined interface (which, in this PR, is aSecretcreated under a certain naming pattern). Theoretically, you should be able to create bootstrapping user-data manually.The point of all this is that MC should not be aware of OSM or depend on it. If this is accepted, I plan to follow up in OSM with adding some of the logic removed here back into it.
In addition, the current MC code does a referential integrity check on an OSP resource - with all the previous explanation this does not make sense anymore anyway (as it's implementation-specific for OSP), but we do not apply referential integrity checks anywhere else for a specific reason: It's against Kubernetes design principles (you won't find this in HPAs referencing a non-existing Deployment, or RBAC role bindings referencing non-existing roles). So, I propose that the check for OSP is removed and replaced with events on
MachineDeployments, highlighting that the referenced OSP does not (yet) exist. But that is for the follow-up PR in OSM.TL;DR: This PR does the following things:
pkg/bootstrapthat relies on some third-party component (such as OSM) creating a user-data bootstrap secret (following the same pattern that OSM applies right now).Note: The OSP annotation magic should move to KKP, I'll follow up on that as well.
Which issue(s) this PR fixes:
Fixes #
What type of PR is this?
/kind cleanup
Special notes for your reviewer:
Does this PR introduce a user-facing change? Then add your Release Note here:
Documentation: