From 0d682ca9ba8f6c4d159c4d68baae4727a672b9a3 Mon Sep 17 00:00:00 2001 From: Sebastian Sch Date: Tue, 14 Feb 2023 16:45:57 +0200 Subject: [PATCH 1/2] use openshift_context instead of internal function in util package Signed-off-by: Sebastian Sch --- cmd/sriov-network-config-daemon/start.go | 27 ++--------- .../sriovnetworkpoolconfig_controller.go | 10 ++--- controllers/sriovoperatorconfig_controller.go | 14 +++--- controllers/suite_test.go | 11 +++-- main.go | 16 +++++-- pkg/daemon/daemon.go | 10 ++--- pkg/daemon/daemon_test.go | 7 +-- pkg/utils/openshift_context.go | 45 ++++++++++++++++++- 8 files changed, 86 insertions(+), 54 deletions(-) diff --git a/cmd/sriov-network-config-daemon/start.go b/cmd/sriov-network-config-daemon/start.go index ff695d234..63ffe84f7 100644 --- a/cmd/sriov-network-config-daemon/start.go +++ b/cmd/sriov-network-config-daemon/start.go @@ -27,9 +27,6 @@ import ( "k8s.io/client-go/util/connrotation" mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" - mcclientset "github.com/openshift/machine-config-operator/pkg/generated/clientset/versioned" - - "sigs.k8s.io/controller-runtime/pkg/client" ) var ( @@ -131,22 +128,9 @@ func runStartCmd(cmd *cobra.Command, args []string) { snclient := snclientset.NewForConfigOrDie(config) kubeclient := kubernetes.NewForConfigOrDie(config) - mcclient := mcclientset.NewForConfigOrDie(config) - openshiftFlavor := utils.OpenshiftFlavorDefault - if utils.ClusterType == utils.ClusterTypeOpenshift { - infraClient, err := client.New(config, client.Options{ - Scheme: scheme.Scheme, - }) - if err != nil { - panic(err) - } - isHypershift, err := utils.IsExternalControlPlaneCluster(infraClient) - if err != nil { - panic(err) - } - if isHypershift { - openshiftFlavor = utils.OpenshiftFlavorHypershift - } + openshiftContext, err := utils.NewOpenshiftContext(config, scheme.Scheme) + if err != nil { + panic(err) } config.Timeout = 5 * time.Second @@ -200,10 +184,7 @@ func runStartCmd(cmd *cobra.Command, args []string) { startOpts.nodeName, snclient, kubeclient, - utils.OpenshiftContext{ - McClient: mcclient, - OpenshiftFlavor: openshiftFlavor, - }, + openshiftContext, exitCh, stopCh, syncCh, diff --git a/controllers/sriovnetworkpoolconfig_controller.go b/controllers/sriovnetworkpoolconfig_controller.go index 061039ad2..635ccb5f4 100644 --- a/controllers/sriovnetworkpoolconfig_controller.go +++ b/controllers/sriovnetworkpoolconfig_controller.go @@ -24,7 +24,8 @@ import ( // SriovNetworkPoolConfigReconciler reconciles a SriovNetworkPoolConfig object type SriovNetworkPoolConfigReconciler struct { client.Client - Scheme *runtime.Scheme + Scheme *runtime.Scheme + OpenshiftContext *utils.OpenshiftContext } //+kubebuilder:rbac:groups=sriovnetwork.openshift.io,resources=sriovnetworkpoolconfigs,verbs=get;list;watch;create;update;patch;delete @@ -43,10 +44,9 @@ type SriovNetworkPoolConfigReconciler struct { func (r *SriovNetworkPoolConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { logger := log.FromContext(ctx).WithValues("sriovnetworkpoolconfig", req.NamespacedName) isHypershift := false - if utils.ClusterType == utils.ClusterTypeOpenshift { - var err error - if isHypershift, err = utils.IsExternalControlPlaneCluster(r.Client); err != nil { - return reconcile.Result{}, err + if r.OpenshiftContext.IsOpenshiftCluster() { + if r.OpenshiftContext.IsHypershift() { + isHypershift = true } logger = logger.WithValues("isHypershift", isHypershift) } diff --git a/controllers/sriovoperatorconfig_controller.go b/controllers/sriovoperatorconfig_controller.go index d7ac0ce09..e7ff7e9b7 100644 --- a/controllers/sriovoperatorconfig_controller.go +++ b/controllers/sriovoperatorconfig_controller.go @@ -44,7 +44,8 @@ import ( // SriovOperatorConfigReconciler reconciles a SriovOperatorConfig object type SriovOperatorConfigReconciler struct { client.Client - Scheme *runtime.Scheme + Scheme *runtime.Scheme + OpenshiftContext *utils.OpenshiftContext } //+kubebuilder:rbac:groups=sriovnetwork.openshift.io,resources=sriovoperatorconfigs,verbs=get;list;watch;create;update;patch;delete @@ -229,12 +230,13 @@ func (r *SriovOperatorConfigReconciler) syncWebhookObjs(ctx context.Context, dc data.Data["CaBundle"] = os.Getenv("WEBHOOK_CA_BUNDLE") data.Data["DevMode"] = os.Getenv("DEV_MODE") data.Data["ImagePullSecrets"] = GetImagePullSecrets() - external, err := utils.IsExternalControlPlaneCluster(r.Client) - if err != nil { - logger.Error(err, "Fail to get control plane topology") - return err + + data.Data["ExternalControlPlane"] = false + if r.OpenshiftContext.IsOpenshiftCluster() { + external := r.OpenshiftContext.IsHypershift() + data.Data["ExternalControlPlane"] = external } - data.Data["ExternalControlPlane"] = external + objs, err := render.RenderDir(path, &data) if err != nil { logger.Error(err, "Fail to render webhook manifests") diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 424a0e31f..9bd666ae7 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -40,6 +40,7 @@ import ( sriovnetworkv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1" constants "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/consts" + "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/utils" //+kubebuilder:scaffold:imports ) @@ -113,14 +114,16 @@ var _ = BeforeSuite(func(done Done) { Expect(err).ToNot(HaveOccurred()) err = (&SriovOperatorConfigReconciler{ - Client: k8sManager.GetClient(), - Scheme: k8sManager.GetScheme(), + Client: k8sManager.GetClient(), + Scheme: k8sManager.GetScheme(), + OpenshiftContext: &utils.OpenshiftContext{OpenshiftFlavor: utils.OpenshiftFlavorDefault}, }).SetupWithManager(k8sManager) Expect(err).ToNot(HaveOccurred()) err = (&SriovNetworkPoolConfigReconciler{ - Client: k8sManager.GetClient(), - Scheme: k8sManager.GetScheme(), + Client: k8sManager.GetClient(), + Scheme: k8sManager.GetScheme(), + OpenshiftContext: &utils.OpenshiftContext{OpenshiftFlavor: utils.OpenshiftFlavorDefault}, }).SetupWithManager(k8sManager) Expect(err).ToNot(HaveOccurred()) diff --git a/main.go b/main.go index 0f22a50ce..f14dee49c 100644 --- a/main.go +++ b/main.go @@ -88,6 +88,12 @@ func main() { os.Exit(1) } + openshiftContext, err := utils.NewOpenshiftContext(restConfig, scheme) + if err != nil { + setupLog.Error(err, "couldn't create openshift context") + os.Exit(1) + } + le := leaderelection.GetLeaderElectionConfig(kubeClient, enableLeaderElection) namespace := os.Getenv("NAMESPACE") @@ -143,15 +149,17 @@ func main() { os.Exit(1) } if err = (&controllers.SriovOperatorConfigReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + OpenshiftContext: openshiftContext, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "SriovOperatorConfig") os.Exit(1) } if err = (&controllers.SriovNetworkPoolConfigReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + OpenshiftContext: openshiftContext, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "SriovNetworkPoolConfig") os.Exit(1) diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go index 773147d45..00fcf0482 100644 --- a/pkg/daemon/daemon.go +++ b/pkg/daemon/daemon.go @@ -69,7 +69,7 @@ type Daemon struct { // kubeClient allows interaction with Kubernetes, including the node we are running on. kubeClient kubernetes.Interface - openshiftContext utils.OpenshiftContext + openshiftContext *utils.OpenshiftContext nodeState *sriovnetworkv1.SriovNetworkNodeState @@ -133,7 +133,7 @@ func New( nodeName string, client snclientset.Interface, kubeClient kubernetes.Interface, - openshiftContext utils.OpenshiftContext, + openshiftContext *utils.OpenshiftContext, exitCh chan<- error, stopCh <-chan struct{}, syncCh <-chan struct{}, @@ -490,7 +490,7 @@ func (dn *Daemon) nodeStateSyncHandler() error { } } } - if utils.ClusterType == utils.ClusterTypeOpenshift && !dn.openshiftContext.IsHypershift() { + if dn.openshiftContext.IsOpenshiftCluster() && !dn.openshiftContext.IsHypershift() { if err = dn.getNodeMachinePool(); err != nil { return err } @@ -507,7 +507,7 @@ func (dn *Daemon) nodeStateSyncHandler() error { <-done } - if utils.ClusterType == utils.ClusterTypeOpenshift && !dn.openshiftContext.IsHypershift() { + if dn.openshiftContext.IsOpenshiftCluster() && !dn.openshiftContext.IsHypershift() { glog.Infof("nodeStateSyncHandler(): pause MCP") if err := dn.pauseMCP(); err != nil { return err @@ -591,7 +591,7 @@ func (dn *Daemon) completeDrain() error { } } - if utils.ClusterType == utils.ClusterTypeOpenshift && !dn.openshiftContext.IsHypershift() { + if dn.openshiftContext.IsOpenshiftCluster() && !dn.openshiftContext.IsHypershift() { glog.Infof("completeDrain(): resume MCP %s", dn.mcpName) pausePatch := []byte("{\"spec\":{\"paused\":false}}") if _, err := dn.openshiftContext.McClient.MachineconfigurationV1().MachineConfigPools().Patch(context.Background(), dn.mcpName, types.MergePatchType, pausePatch, metav1.PatchOptions{}); err != nil { diff --git a/pkg/daemon/daemon_test.go b/pkg/daemon/daemon_test.go index a1b3fc45a..15f54470f 100644 --- a/pkg/daemon/daemon_test.go +++ b/pkg/daemon/daemon_test.go @@ -11,7 +11,6 @@ import ( "github.com/k8snetworkplumbingwg/sriov-network-operator/test/util/fakefilesystem" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - fakemcclientset "github.com/openshift/machine-config-operator/pkg/generated/clientset/versioned/fake" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" fakek8s "k8s.io/client-go/kubernetes/fake" @@ -100,7 +99,6 @@ var _ = Describe("Config Daemon", func() { kubeClient := fakek8s.NewSimpleClientset(&FakeSupportedNicIDs, &SriovDevicePluginPod) client := fakesnclientset.NewSimpleClientset() - mcClient := fakemcclientset.NewSimpleClientset() err = sriovnetworkv1.InitNicIDMap(kubeClient, namespace) Expect(err).ToNot(HaveOccurred()) @@ -108,10 +106,7 @@ var _ = Describe("Config Daemon", func() { sut = New("test-node", client, kubeClient, - utils.OpenshiftContext{ - McClient: mcClient, - OpenshiftFlavor: utils.OpenshiftFlavorDefault, - }, + &utils.OpenshiftContext{IsOpenShiftCluster: false, OpenshiftFlavor: ""}, exitCh, stopCh, syncCh, diff --git a/pkg/utils/openshift_context.go b/pkg/utils/openshift_context.go index 4ffd1b127..44a5b5e41 100644 --- a/pkg/utils/openshift_context.go +++ b/pkg/utils/openshift_context.go @@ -1,6 +1,11 @@ package utils -import mcclientset "github.com/openshift/machine-config-operator/pkg/generated/clientset/versioned" +import ( + mcclientset "github.com/openshift/machine-config-operator/pkg/generated/clientset/versioned" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/pkg/client" +) // OpenshiftFlavor holds metadata about the type of Openshift environment the operator is in. type OpenshiftFlavor string @@ -16,10 +21,48 @@ const ( type OpenshiftContext struct { // McClient is a client for MachineConfigs in an Openshift environment McClient mcclientset.Interface + + // IsOpenShiftCluster boolean to point out if the cluster is an OpenShift cluster + IsOpenShiftCluster bool + // OpenshiftFlavor holds metadata about the type of Openshift environment the operator is in. OpenshiftFlavor OpenshiftFlavor } +func NewOpenshiftContext(config *rest.Config, scheme *runtime.Scheme) (*OpenshiftContext, error) { + if ClusterType != ClusterTypeOpenshift { + return &OpenshiftContext{nil, false, ""}, nil + } + + mcclient, err := mcclientset.NewForConfig(config) + if err != nil { + return nil, err + } + + openshiftFlavor := OpenshiftFlavorDefault + infraClient, err := client.New(config, client.Options{ + Scheme: scheme, + }) + if err != nil { + return nil, err + } + + isHypershift, err := IsExternalControlPlaneCluster(infraClient) + if err != nil { + return nil, err + } + + if isHypershift { + openshiftFlavor = OpenshiftFlavorHypershift + } + + return &OpenshiftContext{mcclient, true, openshiftFlavor}, nil +} + +func (c OpenshiftContext) IsOpenshiftCluster() bool { + return c.IsOpenShiftCluster +} + func (c OpenshiftContext) IsHypershift() bool { return c.OpenshiftFlavor == OpenshiftFlavorHypershift } From 095dc66bcb1899e112a45f45a4247a5a0b34bb39 Mon Sep 17 00:00:00 2001 From: Sebastian Sch Date: Tue, 21 Feb 2023 16:10:18 +0200 Subject: [PATCH 2/2] small change to make IDE validate to pass before this change my IDE was complaining that the function is not right ``` Cannot use 'f' (type func()) as the type DialFunc ``` Signed-off-by: Sebastian Sch --- cmd/sriov-network-config-daemon/start.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/sriov-network-config-daemon/start.go b/cmd/sriov-network-config-daemon/start.go index 63ffe84f7..0a3ab4416 100644 --- a/cmd/sriov-network-config-daemon/start.go +++ b/cmd/sriov-network-config-daemon/start.go @@ -202,7 +202,8 @@ func updateDialer(clientConfig *rest.Config) (func(), error) { if clientConfig.Transport != nil || clientConfig.Dial != nil { return nil, fmt.Errorf("there is already a transport or dialer configured") } - d := connrotation.NewDialer((&net.Dialer{Timeout: 30 * time.Second, KeepAlive: 30 * time.Second}).DialContext) + f := &net.Dialer{Timeout: 30 * time.Second, KeepAlive: 30 * time.Second} + d := connrotation.NewDialer(f.DialContext) clientConfig.Dial = d.DialContext return d.CloseAll, nil }