MGMT-12426: Automatically register cluster with installed MCE #4673
Conversation
Add support for automatically registering the cluster as an MCE managed cluster. Note that the MCE installation part in this patch is just a proof of concept, without configuration, without checking of requirements and without tests, only with the minimum required to verify that it is possible to register the cluster with MCE as part of the installation. That actual implementation will be part of AGENT-381. Related: https://issues.redhat.com/browse/MGMT-12426 Related: https://issues.redhat.com/browse/AGENT-381 Related: https://access.redhat.com/documentation/en-us/red_hat_advanced_cluster_management_for_kubernetes/2.6/html-single/multicluster_engine/index Signed-off-by: Juan Hernandez <juan.hernandez@redhat.com>
openshift-ci
bot
added
the
size/L
openshift-ci
bot
added
the
api-review
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jhernand 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 |
openshift-ci
bot
added
the
approved
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #4673 +/- ##
==========================================
+ Coverage 67.02% 67.06% +0.04%
==========================================
Files 200 200
Lines 28559 28585 +26
==========================================
+ Hits 19141 19171 +30
+ Misses 7679 7677 -2
+ Partials 1739 1737 -2
|
openshift-ci
bot
added
the
do-not-merge/work-in-progress
openshift-merge-robot
added
the
needs-rebase
|
@jhernand: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
| @@ -32,7 +33,18 @@ type Options struct { | |||
|
|
|||
| // NewManager creates new instance of an Operator Manager | |||
| func NewManager(log logrus.FieldLogger, manifestAPI manifestsapi.ManifestsAPI, options Options, objectHandler s3wrapper.API, extracter oc.Extracter) *Manager { | |||
| return NewManagerWithOperators(log, manifestAPI, options, objectHandler, lso.NewLSOperator(), odf.NewOcsOperator(log), odf.NewOdfOperator(log, extracter), cnv.NewCNVOperator(log, options.CNVConfig, extracter), lvm.NewLvmOperator(log, extracter)) | |||
| return NewManagerWithOperators( | |||
There was a problem hiding this comment.
seeing how this function is getting bigger and bigger maybe we should consider having a struct parameter?
There was a problem hiding this comment.
It is a variadic function, with only five parameters:
Anyhow I'd say that more than three parameters is probably too many. No objection to introduce a struct parameter or some other mechanism.
| @@ -0,0 +1,4 @@ | |||
| package mce | |||
|
|
|||
| type Config struct { | |||
There was a problem hiding this comment.
Not really needed. Other operators have some fields and I wanted to preserve the pattern in case we want to add something.
This PR introduces an MCE operator that can be deployed as part of a cluster installation. The operator is depended on the LVM operator, I.e. LVMS is also deployed during cluster installation. The suggested flow here is just for demonstration purposes to illustrate the simplicity of using LVMS as a local persistent storage solution for an effort[1] of a new MCE operator. So we have just the basic flow here without validations/tests/etc, as inspired by the original PR[2] :) Note: LVMS is currently limited for SNO, this PR[3] is removing that limitation to support mulit-node clusters as well. [1] https://issues.redhat.com/browse/MGMT-12186 [2] openshift#4673 [3] openshift#4936
This PR introduces an MCE operator that can be deployed as part of a cluster installation. The operator is depended on the LVM operator, I.e. LVMS is also deployed during cluster installation. The suggested flow here is just for demonstration purposes to illustrate the simplicity of using LVMS as a local persistent storage solution for an effort[1] of a new MCE operator. So we have just the basic flow here without validations/tests/etc, as inspired by the original PR[2] :) Note: LVMS is currently limited for SNO, this PR[3] is removing that limitation to support multi-node clusters as well. [1] https://issues.redhat.com/browse/MGMT-12186 [2] openshift#4673 [3] openshift#4936
|
@jhernand: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
@jhernand looks like an old PR, closing, feel free to reopen if needed |
Add support for automatically registering the cluster as an MCE managed cluster.
Note that the MCE installation part in this patch is just a proof of concept, without configuration, without checking of requirements and without tests, only with the minimum required to verify that it is possible to register the cluster with MCE as part of the installation. That actual implementation will be part of AGENT-381.
Related: https://issues.redhat.com/browse/MGMT-12426
Related: https://issues.redhat.com/browse/AGENT-381
Related: https://access.redhat.com/documentation/en-us/red_hat_advanced_cluster_management_for_kubernetes/2.6/html-single/multicluster_engine/index
List all the issues related to this PR
What environments does this code impact?
How was this code tested?
Checklist
docs, README, etc)Reviewers Checklist