Ensure KataConfigPoolSelector is used for PeerPodConfig CRD #305
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
The peerpods config enablement is split into two parts - one part deals with creation of MachineConfigs before "kata-oc" MCP creation and another part deals with creation of PeerPodConfig CRD, runtimeclass and other misc configs after "kata-oc" MCP has updated. The reason for this split is to ensure that MachineConfigs are created before "kata-oc" creation so as to optimise the number of reboots required when applying MachineConfigs. Further, the relevant node labels are only available when "kata-oc" MCP is updated, thereby making it available for the creation of PeerPodConfig CRD Signed-off-by: Pradipta Banerjee <pradipta.banerjee@gmail.com>
cpmeadors
previously approved these changes
May 9, 2023
pmores
reviewed
May 9, 2023
| if err != nil { | ||
| r.Log.Info("Enabling peerpodconfig CR, runtimeclass etc", "err", err) | ||
| // Give sometime for the error to go away before reconciling again | ||
| return reconcile.Result{Requeue: true, RequeueAfter: 15 * time.Second}, err |
There was a problem hiding this comment.
I would suggest that in a situation where we know nothing about the nature or cause of an error, a more appropriate handling would be to rely on the exponential back-off provided by reconcile.Result{Requeue: true} instead of a specific timeout which, considering the lack of information, has to be basically arbitrary.
But this will be a subject of a future PR to unify our reconciler return values so no need to change it now.
pmores
approved these changes
May 9, 2023
jensfr
approved these changes
May 9, 2023
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.
The peerpods config enablement is split into two parts - one part deals with creation of MachineConfigs before "kata-oc" MCP creation and another part deals with creation of PeerPodConfig CRD, runtimeclass and other misc configs after "kata-oc" MCP has updated.
The reason for this split is to ensure that MachineConfigs are created before "kata-oc" creation so as to optimise the number of reboots required when applying MachineConfigs. Further, the relevant node labels are only available when "kata-oc" MCP is updated, thereby making it available for the creation of PeerPodConfig CRD
- Description of the problem which is fixed/What is the use case
The cloud-api-adaptor daemonset was not taking into account the KataConfigPoolSelector
Fixes: #KATA-1873
- What I did
Ensured KataConfigPoolSelector is made available to the PeerPodConfig CRD which is then used by the PeerPodConfig controller to start the cloud-api-adaptor daemonset on the proper nodes
- How to verify it
Create a KataConfig with custom KataConfigPoolSelector and ensure
enablePeerPods: true. Verify that the cloud-api-adaptor pods are created only on the nodes matched via the KataConfigPoolSelector- Description for the changelog
Ensure KataConfigPoolSelector is used for PeerPodConfig CRD