✨ Initialize hub with aws irsa #465
Conversation
pkg/cmd/init/exec.go
Outdated
| klog.Errorf("unable to load hub cluster kubeconfig: %v", err) | ||
| return nil, err | ||
| } | ||
| hubClusterArn := rawConfig.Contexts[rawConfig.CurrentContext].Cluster |
There was a problem hiding this comment.
why the arn is the cluster name? is it alway true on EKS?
There was a problem hiding this comment.
Adding another option flag called --hub-cluster-arn which can be used in tests as well as in case the hubclusterArn is missing in kubeconfig.
jaswalkiranavtar
force-pushed
the
initialize-hub-with-aws-irsa
branch
from
b4689d5 to
06dd0f0
Compare
|
Merge this PR first: open-cluster-management-io/ocm#810 And update the go.mod which latest commit sha of ocm and then merge this. |
| @@ -78,6 +81,9 @@ func NewCmd(clusteradmFlags *genericclioptionsclusteradm.ClusteradmFlags, stream | |||
| _ = clusterManagerSet.SetAnnotation("singleton-name", "singletonSet", []string{}) | |||
| o.Helm.AddFlags(singletonSet) | |||
| cmd.Flags().AddFlagSet(singletonSet) | |||
| cmd.Flags().StringArrayVar(&o.registrationAuth, "registration-auth", []string{}, "The type of authentication to use for registering and authenticating with hub, this flag can be repeated to specify multiple authentication types.") | |||
| cmd.Flags().StringVar(&o.hubClusterArn, "hub-cluster-arn", "", | |||
There was a problem hiding this comment.
I think you would need to validate this value
There was a problem hiding this comment.
Thr validation for this is already added to cluster-manager spec and the validation error is already propagated back to the clusteradm consumer even now.
Is that sufficient?
pkg/cmd/init/cmd.go
Outdated
| @@ -78,6 +81,9 @@ func NewCmd(clusteradmFlags *genericclioptionsclusteradm.ClusteradmFlags, stream | |||
| _ = clusterManagerSet.SetAnnotation("singleton-name", "singletonSet", []string{}) | |||
| o.Helm.AddFlags(singletonSet) | |||
| cmd.Flags().AddFlagSet(singletonSet) | |||
| cmd.Flags().StringArrayVar(&o.registrationAuth, "registration-auth", []string{}, "The type of authentication to use for registering and authenticating with hub, this flag can be repeated to specify multiple authentication types.") | |||
There was a problem hiding this comment.
it is better to validate the flag, only valid auth drvier is allowed. In the message, also should give the allowed value.
| @@ -353,3 +359,27 @@ func (o *Options) deploySingletonControlplane(kubeClient kubernetes.Interface) e | |||
| } | |||
| return nil | |||
| } | |||
|
|
|||
| func getRegistrationDrivers(o *Options) ([]operatorv1.RegistrationDriverHub, error) { | |||
There was a problem hiding this comment.
could we make it simpler? we should validate and return error when a driver name is not allowed in advance.
for _, driver := range o.registrationAuth:
registrationDriver := operatorv1.RegistrationDriverHub{AuthType: driver}
if driver == "awisra" {
...
}
registrationDrivers = append(registrationDrivers, registrationDriver)
|
Also commented the e2e test with a TODO |
jaswalkiranavtar
force-pushed
the
initialize-hub-with-aws-irsa
branch
from
f5fa2b6 to
b435511
Compare
Signed-off-by: Gaurav Jaswal <[email protected]>
jaswalkiranavtar
force-pushed
the
initialize-hub-with-aws-irsa
branch
from
b435511 to
81d5c1e
Compare
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jaswalkiranavtar, qiujian16 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-merge-bot
bot
merged commit a756dd6
into
open-cluster-management-io:main
open-cluster-management-io#465) Signed-off-by: Gaurav Jaswal <[email protected]> Co-authored-by: EmilyL <[email protected]>
Summary
Related issue(s)
Fixes # open-cluster-management-io/ocm#514