[release-4.18][manual] objstate: rte: isolate platform-specific code #1152
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
extract helper to update the state from either NodeGroups or MachineConfigPools into helpers. The goal here is to isolate this code to make the main calling functions more targeted and to unblock future refactoring. Signed-off-by: Francesco Romani <[email protected]> (cherry picked from commit b249228)
openshift-ci
bot
added
the
approved
extract helper to fetch the state from either NodeGroups or MachineConfigPools into helpers. The goal here is to isolate this code to make the main calling functions more targeted and to unblock future refactoring. Add helpers to inject parameters. This helps making the objectstate flavours all consistent with each other; previously `rte` was the outlier and the only variant with extra parameters. This require the constructor function to return a pointer to a struct (not a struct). Update all the variants for consistency. Signed-off-by: Francesco Romani <[email protected]> (cherry picked from commit b4964b1)
move helpers in per-mode files. This is meant to keep the filesize at bay and to enable future code extraction. No intended change in behavior. Signed-off-by: Francesco Romani <[email protected]> (cherry picked from commit 6668e12)
the helpers we extracted in previous commits still share some state; pack them in a struct, which makes also nicer to consume them from the caller site(s), acting almost like a type. In the (far?) future, when we will move completely to nodegroups (if ever), this code should be dissolved back in the callers. Signed-off-by: Francesco Romani <[email protected]> (cherry picked from commit 860ffd1)
ffromani
force-pushed
the
plat-cleanup-objstate-4.18
branch
from
a63b7c8 to
1cb525c
Compare
Tal-or
approved these changes
Jan 13, 2025
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ffromani, Tal-or 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 |
|
/test ci-e2e-install-hypershift |
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.
isolate per-platform specific code in their own types and source code, to reduce the clutter without intentional changes of behavior.
The idea is to gradually move towards full isolation and containment of the platform-specific code, and this change is a step in that direction.
Actually, we dopn't really care about the platform but rather about how we find the nodes, but the method (poolName vs machineconfigpool) is still tied to the platform, hence the ambiguity.
In the long run we want to support only the newer pool approach introduced with HCP.
manual backport of #1114