From a86872b5f73a537a30670facc076d91d743c4f21 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Wed, 23 Feb 2022 12:18:24 +0000 Subject: [PATCH] Set API Version based on flag when discovery won't work --- main.go | 15 ++++++++------ manifests/04-deployment.yaml | 2 ++ pkg/controller/controller.go | 16 ++++++++------- pkg/machinehandler/machinehandler.go | 23 +++++++++++++-------- pkg/machinehandler/machinehandler_test.go | 25 +++++++++++++---------- 5 files changed, 48 insertions(+), 33 deletions(-) diff --git a/main.go b/main.go index 97155b1b9..f8cbfb254 100644 --- a/main.go +++ b/main.go @@ -45,7 +45,8 @@ const ( func main() { var cliConfig string - var APIGroup string + var apiGroup string + var apiVersion string var managementKubeConfigPath string var machineNamespace string var workloadKubeConfigPath string @@ -61,7 +62,8 @@ func main() { klog.InitFlags(flagSet) flagSet.StringVar(&cliConfig, "config", "", "CLI config") - flagSet.StringVar(&APIGroup, "apigroup", "machine.openshift.io", "API group for machines, defaults to machine.openshift.io") + flagSet.StringVar(&apiGroup, "apigroup", "machine.openshift.io", "API group for machines, defaults to machine.openshift.io") + flagSet.StringVar(&apiVersion, "apiversion", "", "API version for machines, will default to the server preferred version if not set") flagSet.StringVar(&managementKubeConfigPath, "management-cluster-kubeconfig", "", "management kubeconfig path,") flagSet.StringVar(&machineNamespace, "machine-namespace", "", "restrict machine operations to a specific namespace, if not set, all machines will be observed in approval decisions") flagSet.StringVar(&workloadKubeConfigPath, "workload-cluster-kubeconfig", "", "workload kubeconfig path") @@ -75,7 +77,7 @@ func main() { flagSet.StringVar(&leaderElectResourceNamespace, "leader-elect-resource-namespace", "openshift-cluster-machine-approver", "the namespace in which the leader election resource will be created.") flagSet.Parse(os.Args[1:]) - if err := validateAPIGroup(APIGroup); err != nil { + if err := validateapiGroup(apiGroup); err != nil { klog.Fatalf(err.Error()) } @@ -164,7 +166,8 @@ func main() { NodeClient: uncachedWorkloadClient, NodeRestCfg: workloadConfig, Config: controller.LoadConfig(cliConfig), - APIGroup: APIGroup, + APIGroup: apiGroup, + APIVersion: apiVersion, }).SetupWithManager(mgr, ctrl.Options{}); err != nil { klog.Fatalf("unable to create CSR controller: %v", err) } @@ -218,9 +221,9 @@ func createClients(managementConfig, workloadConfig *rest.Config) (*client.Clien return &managementClient, &workloadClient, nil } -func validateAPIGroup(apiGroup string) error { +func validateapiGroup(apiGroup string) error { if apiGroup != capiGroup && apiGroup != mapiGroup { - return fmt.Errorf("unsupported APIGroup %s, allowed values %s, %s", apiGroup, capiGroup, mapiGroup) + return fmt.Errorf("unsupported apiGroup %s, allowed values %s, %s", apiGroup, capiGroup, mapiGroup) } return nil diff --git a/manifests/04-deployment.yaml b/manifests/04-deployment.yaml index 50f1f5134..e5f97381c 100644 --- a/manifests/04-deployment.yaml +++ b/manifests/04-deployment.yaml @@ -61,6 +61,8 @@ spec: command: ["/usr/bin/machine-approver"] args: - "--config=/var/run/configmaps/config/config.yaml" + - "--apigroup=machine.openshift.io" + - "--apiversion=v1beta1" - "-v=2" - "--logtostderr" - "--leader-elect=true" diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index ecca0e5e9..1b114f490 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -40,8 +40,9 @@ type CertificateApprover struct { MachineRestCfg *rest.Config MachineNamespace string - Config ClusterMachineApproverConfig - APIGroup string + Config ClusterMachineApproverConfig + APIGroup string + APIVersion string } func (m *CertificateApprover) SetupWithManager(mgr ctrl.Manager, options controller.Options) error { @@ -122,11 +123,12 @@ func (m *CertificateApprover) Reconcile(ctx context.Context, req ctrl.Request) ( } machineHandler := &machinehandlerpkg.MachineHandler{ - Client: m.MachineClient, - Config: m.MachineRestCfg, - Ctx: ctx, - APIGroup: m.APIGroup, - Namespace: m.MachineNamespace, + Client: m.MachineClient, + Config: m.MachineRestCfg, + Ctx: ctx, + APIGroup: m.APIGroup, + APIVersion: m.APIVersion, + Namespace: m.MachineNamespace, } machines, err := machineHandler.ListMachines() diff --git a/pkg/machinehandler/machinehandler.go b/pkg/machinehandler/machinehandler.go index 00897b4ab..e7dcecb30 100644 --- a/pkg/machinehandler/machinehandler.go +++ b/pkg/machinehandler/machinehandler.go @@ -21,11 +21,12 @@ var ( ) type MachineHandler struct { - APIGroup string - Client client.Client - Config *rest.Config - Ctx context.Context - Namespace string + APIGroup string + APIVersion string + Client client.Client + Config *rest.Config + Ctx context.Context + Namespace string } type Machine struct { @@ -39,16 +40,20 @@ type MachineStatus struct { // ListMachines list all machines using given client func (m *MachineHandler) ListMachines() ([]Machine, error) { - APIVersion, err := m.getAPIGroupPreferredVersion() - if err != nil { - return nil, err + apiVersion := m.APIVersion + if apiVersion == "" { + var err error + apiVersion, err = m.getAPIGroupPreferredVersion() + if err != nil { + return nil, err + } } unstructuredMachineList := &unstructured.UnstructuredList{} unstructuredMachineList.SetGroupVersionKind(schema.GroupVersionKind{ Group: m.APIGroup, Kind: "MachineList", - Version: APIVersion, + Version: apiVersion, }) listOpts := make([]client.ListOption, 0) if m.Namespace != "" { diff --git a/pkg/machinehandler/machinehandler_test.go b/pkg/machinehandler/machinehandler_test.go index 8b5ce7d84..e3e727ad7 100644 --- a/pkg/machinehandler/machinehandler_test.go +++ b/pkg/machinehandler/machinehandler_test.go @@ -9,6 +9,7 @@ import ( "testing" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -108,13 +109,14 @@ func Test_authorizeCSR(t *testing.T) { capiMachine2 := createUnstructuredMachine("cluster.x-k8s.io/v1alpha4", "capi-machine2", "capi-machine2", "10.0.128.124", "ip-10-0-128-124.ec2.internal") ocpMachine1 := createUnstructuredMachine("machine.openshift.io/v1beta1", "ocp-machine1", "ocp-machine1", "10.0.172.123", "ip-10-0-172-123.ec2.internal") ocpmachine2 := createUnstructuredMachine("machine.openshift.io/v1beta1", "ocp-machine2", "ocp-machine2", "10.0.172.124", "ip-10-0-172-124.ec2.internal") - cl := fake.NewClientBuilder().WithObjects(capiMachine1, capiMachine2, ocpMachine1, ocpmachine2).Build() + cl := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(capiMachine1, capiMachine2, ocpMachine1, ocpmachine2).Build() type args struct { - apiGroup string - client client.Client - config *rest.Config - ctx context.Context - namespace string + apiGroup string + apiVersion string + client client.Client + config *rest.Config + ctx context.Context + namespace string } tests := []struct { @@ -155,11 +157,12 @@ func Test_authorizeCSR(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { handler := MachineHandler{ - APIGroup: tt.args.apiGroup, - Client: tt.args.client, - Config: tt.args.config, - Ctx: tt.args.ctx, - Namespace: tt.args.namespace, + APIGroup: tt.args.apiGroup, + APIVersion: tt.args.apiVersion, + Client: tt.args.client, + Config: tt.args.config, + Ctx: tt.args.ctx, + Namespace: tt.args.namespace, } machines, err := handler.ListMachines() if (err != nil) != tt.wantErr {