Merge pull request #2741 from sebsoto/fixContext

WINC-1378: Handle interrupt signals with the proper use of contexts

Author: openshift-merge-bot[bot]

cmd/daemon/bootstrap.go

@@ -19,11 +19,10 @@ limitations under the License.
 package main
 
 import (
-	"context"
-
 	"github.com/spf13/cobra"
 	"k8s.io/client-go/tools/clientcmd"
 	"k8s.io/klog/v2"
+	ctrl "sigs.k8s.io/controller-runtime"
 
 	"github.com/openshift/windows-machine-config-operator/pkg/daemon/controller"
 )
@@ -53,7 +52,7 @@ func runBootstrapCmd(cmd *cobra.Command, args []string) {
 	if err != nil {
 		klog.Exitf("error building config: %s", err.Error())
 	}
-	sc, err := controller.NewServiceController(context.TODO(), "", namespace, controller.Options{Config: cfg})
+	sc, err := controller.NewServiceController(ctrl.SetupSignalHandler(), "", namespace, controller.Options{Config: cfg})
 	if err != nil {
 		klog.Exitf("error creating Service Controller: %s", err.Error())
 	}

cmd/operator/main.go

@@ -1,7 +1,6 @@
 package main
 
 import (
-	"context"
 	"flag"
 	"fmt"
 	"os"
@@ -106,14 +105,15 @@ func main() {
 	}
 
 	// get cluster configuration
-	clusterConfig, err := cluster.NewConfig(cfg)
+	ctx := ctrl.SetupSignalHandler()
+	clusterConfig, err := cluster.NewConfig(ctx, cfg)
 	if err != nil {
 		setupLog.Error(err, "failed to get cluster configuration")
 		os.Exit(1)
 	}
 
 	// validate cluster for required configurations
-	if err := clusterConfig.Validate(); err != nil {
+	if err := clusterConfig.Validate(ctx); err != nil {
 		setupLog.Error(err, "failed to validate required cluster configuration")
 		os.Exit(1)
 	}
@@ -146,7 +146,6 @@ func main() {
 		os.Exit(1)
 	}
 
-	ctx := context.TODO()
 	// Become the leader before proceeding
 	err = leader.Become(ctx, "windows-machine-config-operator-lock")
 	if err != nil {
@@ -229,16 +228,16 @@ func main() {
 		setupLog.Error(err, "unable to create Secret reconciler")
 		os.Exit(1)
 	}
-	if err = secretReconciler.SetupWithManager(mgr); err != nil {
+	if err = secretReconciler.SetupWithManager(ctx, mgr); err != nil {
 		setupLog.Error(err, "unable to create Secret controller")
 		os.Exit(1)
 	}
-	if err := secretReconciler.RemoveInvalidAnnotationsFromLinuxNodes(mgr.GetConfig()); err != nil {
+	if err := secretReconciler.RemoveInvalidAnnotationsFromLinuxNodes(ctx, mgr.GetConfig()); err != nil {
 		setupLog.Error(err, "error removing invalid annotations from Linux nodes")
 	}
 
 	proxyEnabled := cluster.IsProxyEnabled()
-	configMapReconciler, err := controllers.NewConfigMapReconciler(mgr, clusterConfig, watchNamespace, proxyEnabled)
+	configMapReconciler, err := controllers.NewConfigMapReconciler(ctx, mgr, clusterConfig, watchNamespace, proxyEnabled)
 	if err != nil {
 		setupLog.Error(err, "unable to create ConfigMap reconciler")
 		os.Exit(1)
@@ -284,7 +283,7 @@ func main() {
 	// controllers are generated by Operator SDK.
 
 	// Create the singleton Windows services ConfigMap
-	if err := configMapReconciler.EnsureServicesConfigMapExists(); err != nil {
+	if err := configMapReconciler.EnsureServicesConfigMapExists(ctx); err != nil {
 		setupLog.Error(err, "error ensuring object exists", "singleton", types.NamespacedName{Namespace: watchNamespace,
 			Name: servicescm.Name})
 		os.Exit(1)
@@ -298,14 +297,14 @@ func main() {
 	// If proxy is enabled, disabled, or edited during WMCO runtime, the WMCO pod will be restarted by OLM. This could
 	// happen in the middle of node configuration, at which the controllers will reconcile once the WMCO pod restarts
 	if proxyEnabled {
-		if err := configMapReconciler.EnsureTrustedCAConfigMapExists(); err != nil {
+		if err := configMapReconciler.EnsureTrustedCAConfigMapExists(ctx); err != nil {
 			setupLog.Error(err, "error ensuring trusted CA ConfigMap exists", "namespace", watchNamespace)
 			os.Exit(1)
 		}
 	}
 
 	setupLog.Info("starting manager")
-	if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil {
+	if err := mgr.Start(ctx); err != nil {
 		setupLog.Error(err, "problem running manager")
 
 		os.Exit(1)

controllers/certificatesigningrequests_controller.go

@@ -80,11 +80,11 @@ func (r *certificateSigningRequestsReconciler) Reconcile(ctx context.Context,
 	_ = r.log.WithValues(CSRController, req.NamespacedName)
 
 	// Prevent WMCO upgrades while CSRs are being processed
-	if err := condition.MarkAsBusy(r.client, r.watchNamespace, r.recorder, CSRController); err != nil {
+	if err := condition.MarkAsBusy(ctx, r.client, r.watchNamespace, r.recorder, CSRController); err != nil {
 		return ctrl.Result{}, err
 	}
 	defer func() {
-		reconcileErr = markAsFreeOnSuccess(r.client, r.watchNamespace, r.recorder, CSRController,
+		reconcileErr = markAsFreeOnSuccess(ctx, r.client, r.watchNamespace, r.recorder, CSRController,
 			result.Requeue, reconcileErr)
 	}()
 
@@ -121,7 +121,7 @@ func (r *certificateSigningRequestsReconciler) reconcileCSR(ctx context.Context,
 			return fmt.Errorf("could not create WMCO CSR Approver: %w", err)
 		}
 
-		return csrApprover.Approve()
+		return csrApprover.Approve(ctx)
 	})
 	if err != nil {
 		// Max retries were hit, or unrelated issue like permissions or a network error

controllers/configmap_controller.go

@@ -90,7 +90,7 @@ type ConfigMapReconciler struct {
 }
 
 // NewConfigMapReconciler returns a pointer to a ConfigMapReconciler
-func NewConfigMapReconciler(mgr manager.Manager, clusterConfig cluster.Config, watchNamespace string,
+func NewConfigMapReconciler(ctx context.Context, mgr manager.Manager, clusterConfig cluster.Config, watchNamespace string,
 	proxyEnabled bool) (*ConfigMapReconciler, error) {
 	clientset, err := kubernetes.NewForConfig(mgr.GetConfig())
 	if err != nil {
@@ -101,7 +101,7 @@ func NewConfigMapReconciler(mgr manager.Manager, clusterConfig cluster.Config, w
 	if err != nil {
 		return nil, err
 	}
-	svcData, err := generateServicesManifest(directClient, clusterConfig.Network().VXLANPort(), clusterConfig.Platform())
+	svcData, err := generateServicesManifest(ctx, directClient, clusterConfig.Network().VXLANPort(), clusterConfig.Platform())
 	if err != nil {
 		return nil, err
 	}
@@ -132,22 +132,22 @@ func (r *ConfigMapReconciler) Reconcile(ctx context.Context,
 
 	var err error
 	// Prevent WMCO upgrades while BYOH nodes are being processed.
-	if err := condition.MarkAsBusy(r.client, r.watchNamespace, r.recorder, ConfigMapController); err != nil {
+	if err := condition.MarkAsBusy(ctx, r.client, r.watchNamespace, r.recorder, ConfigMapController); err != nil {
 		return ctrl.Result{}, err
 	}
 	defer func() {
-		reconcileErr = markAsFreeOnSuccess(r.client, r.watchNamespace, r.recorder, ConfigMapController,
+		reconcileErr = markAsFreeOnSuccess(ctx, r.client, r.watchNamespace, r.recorder, ConfigMapController,
 			result.Requeue, reconcileErr)
 	}()
 
 	// Create a new signer using the private key that the instances will be configured with
-	r.signer, err = signer.Create(kubeTypes.NamespacedName{Namespace: r.watchNamespace,
+	r.signer, err = signer.Create(ctx, kubeTypes.NamespacedName{Namespace: r.watchNamespace,
 		Name: secrets.PrivateKeySecret}, r.client)
 	if err != nil {
 		return ctrl.Result{}, fmt.Errorf("unable to create signer from private key secret: %w", err)
 	}
 
-	servicesManifest, err := generateServicesManifest(r.client, r.VXLANPort, r.platform)
+	servicesManifest, err := generateServicesManifest(ctx, r.client, r.VXLANPort, r.platform)
 	if err != nil {
 		return ctrl.Result{}, err
 	}
@@ -273,23 +273,23 @@ func (r *ConfigMapReconciler) reconcileNodes(ctx context.Context, windowsInstanc
 
 	r.log.Info("processing", "instances in", wiparser.InstanceConfigMap)
 	// For each instance, ensure that it is configured into a node
-	if err := r.ensureInstancesAreUpToDate(instances); err != nil {
+	if err := r.ensureInstancesAreUpToDate(ctx, instances); err != nil {
 		r.recorder.Eventf(windowsInstances, core.EventTypeWarning, "InstanceSetupFailure", err.Error())
 		return err
 	}
 
 	// Ensure that only instances currently specified by the ConfigMap are joined to the cluster as nodes
-	if err = r.deconfigureInstances(instances, nodes); err != nil {
+	if err = r.deconfigureInstances(ctx, instances, nodes); err != nil {
 		return fmt.Errorf("error removing undesired nodes from cluster: %w", err)
 	}
 
 	return nil
 }
 
 // ensureInstancesAreUpToDate configures all instances that require configuration
-func (r *ConfigMapReconciler) ensureInstancesAreUpToDate(instances []*instance.Info) error {
+func (r *ConfigMapReconciler) ensureInstancesAreUpToDate(ctx context.Context, instances []*instance.Info) error {
 	// Get private key to encrypt instance usernames
-	privateKeyBytes, err := secrets.GetPrivateKey(kubeTypes.NamespacedName{Namespace: r.watchNamespace,
+	privateKeyBytes, err := secrets.GetPrivateKey(ctx, kubeTypes.NamespacedName{Namespace: r.watchNamespace,
 		Name: secrets.PrivateKeySecret}, r.client)
 	if err != nil {
 		return err
@@ -304,7 +304,7 @@ func (r *ConfigMapReconciler) ensureInstancesAreUpToDate(instances []*instance.I
 		if err != nil {
 			return fmt.Errorf("unable to encrypt username for instance %s: %w", instanceInfo.Address, err)
 		}
-		err = r.ensureInstanceIsUpToDate(instanceInfo, map[string]string{BYOHLabel: "true", nodeconfig.WorkerLabel: ""},
+		err = r.ensureInstanceIsUpToDate(ctx, instanceInfo, map[string]string{BYOHLabel: "true", nodeconfig.WorkerLabel: ""},
 			map[string]string{UsernameAnnotation: encryptedUsername})
 		if err != nil {
 			// It is better to return early like this, instead of trying to configure as many instances as possible in a
@@ -321,7 +321,7 @@ func (r *ConfigMapReconciler) ensureInstancesAreUpToDate(instances []*instance.I
 
 // deconfigureInstances removes all BYOH nodes that are not specified in the given instances slice, and
 // deconfigures the instances associated with them. The nodes parameter should be a list of all Windows BYOH nodes.
-func (r *ConfigMapReconciler) deconfigureInstances(instances []*instance.Info, nodes *core.NodeList) error {
+func (r *ConfigMapReconciler) deconfigureInstances(ctx context.Context, instances []*instance.Info, nodes *core.NodeList) error {
 	windowsInstances := &core.ConfigMap{ObjectMeta: meta.ObjectMeta{Name: wiparser.InstanceConfigMap,
 		Namespace: r.watchNamespace}}
 	for _, node := range nodes.Items {
@@ -331,7 +331,7 @@ func (r *ConfigMapReconciler) deconfigureInstances(instances []*instance.Info, n
 		}
 
 		// no instance found in the provided list, remove the node from the cluster
-		if err := r.deconfigureInstance(&node); err != nil {
+		if err := r.deconfigureInstance(ctx, &node); err != nil {
 			return fmt.Errorf("unable to deconfigure instance with node %s: %w", node.GetName(), err)
 		}
 		r.recorder.Eventf(windowsInstances, core.EventTypeNormal, "InstanceTeardown",
@@ -451,12 +451,12 @@ func (r *ConfigMapReconciler) createServicesConfigMap(ctx context.Context) (*cor
 // createServicesConfigMapOnBootup creates a valid ServicesConfigMap
 // ConfigMapReconciler.createServicesConfigMap() cannot be used in its stead as the cache has not been
 // populated yet, which is why the typed client is used here as it calls the API server directly.
-func (r *ConfigMapReconciler) createServicesConfigMapOnBootup() error {
+func (r *ConfigMapReconciler) createServicesConfigMapOnBootup(ctx context.Context) error {
 	windowsServices, err := servicescm.Generate(servicescm.Name, r.watchNamespace, r.servicesManifest)
 	if err != nil {
 		return err
 	}
-	cm, err := r.k8sclientset.CoreV1().ConfigMaps(r.watchNamespace).Create(context.TODO(), windowsServices,
+	cm, err := r.k8sclientset.CoreV1().ConfigMaps(r.watchNamespace).Create(ctx, windowsServices,
 		meta.CreateOptions{})
 	if err != nil {
 		return err
@@ -466,13 +466,13 @@ func (r *ConfigMapReconciler) createServicesConfigMapOnBootup() error {
 }
 
 // EnsureServicesConfigMapExists ensures that the ServicesConfigMap is present and valid on operator bootup
-func (r *ConfigMapReconciler) EnsureServicesConfigMapExists() error {
-	windowsServices, err := r.k8sclientset.CoreV1().ConfigMaps(r.watchNamespace).Get(context.TODO(), servicescm.Name,
+func (r *ConfigMapReconciler) EnsureServicesConfigMapExists(ctx context.Context) error {
+	windowsServices, err := r.k8sclientset.CoreV1().ConfigMaps(r.watchNamespace).Get(ctx, servicescm.Name,
 		meta.GetOptions{})
 	if err != nil {
 		if k8sapierrors.IsNotFound(err) {
 			// If ConfigMap is not found, create it and return
-			return r.createServicesConfigMapOnBootup()
+			return r.createServicesConfigMapOnBootup(ctx)
 		}
 		return err
 	}
@@ -484,13 +484,13 @@ func (r *ConfigMapReconciler) EnsureServicesConfigMapExists() error {
 	}
 
 	// Delete and re-create the ConfigMap with the proper values
-	if err = r.k8sclientset.CoreV1().ConfigMaps(r.watchNamespace).Delete(context.TODO(), windowsServices.Name,
+	if err = r.k8sclientset.CoreV1().ConfigMaps(r.watchNamespace).Delete(ctx, windowsServices.Name,
 		meta.DeleteOptions{}); err != nil {
 		return err
 	}
 	r.log.Info("Deleted invalid resource", "ConfigMap",
 		kubeTypes.NamespacedName{Namespace: r.watchNamespace, Name: servicescm.Name})
-	return r.createServicesConfigMapOnBootup()
+	return r.createServicesConfigMapOnBootup(ctx)
 }
 
 // createProxyCertsCM creates the trusted CA ConfigMap with the expected spec
@@ -529,7 +529,7 @@ func (r *ConfigMapReconciler) ensureTrustedCABundleInNodes(ctx context.Context)
 
 // ensureTrustedCABundleInNodes places the trusted CA bundle data into a file on the given node
 func (r *ConfigMapReconciler) ensureTrustedCABundleInNode(ctx context.Context, node core.Node) error {
-	winInstance, err := r.instanceFromNode(&node)
+	winInstance, err := r.instanceFromNode(ctx, &node)
 	if err != nil {
 		return err
 	}
@@ -538,7 +538,7 @@ func (r *ConfigMapReconciler) ensureTrustedCABundleInNode(ctx context.Context, n
 	if err != nil {
 		return fmt.Errorf("failed to create new nodeconfig: %w", err)
 	}
-	return nc.SyncTrustedCABundle()
+	return nc.SyncTrustedCABundle(ctx)
 }
 
 // ensureProxyCertsCMIsValid ensures the trusted CA ConfigMap has the expected injection request. Patches the object if not.
@@ -555,7 +555,7 @@ func (r *ConfigMapReconciler) ensureProxyCertsCMIsValid(ctx context.Context, inj
 		return fmt.Errorf("unable to generate patch request body for label %s: %w", InjectionRequestLabel, err)
 	}
 
-	if _, err = r.k8sclientset.CoreV1().ConfigMaps(r.watchNamespace).Patch(context.TODO(),
+	if _, err = r.k8sclientset.CoreV1().ConfigMaps(r.watchNamespace).Patch(ctx,
 		certificates.ProxyCertsConfigMap, kubeTypes.JSONPatchType, patchData, meta.PatchOptions{}); err != nil {
 		return fmt.Errorf("unable to apply patch %s to resource %s/%s: %w", patchData, r.watchNamespace,
 			certificates.ProxyCertsConfigMap, err)
@@ -566,16 +566,16 @@ func (r *ConfigMapReconciler) ensureProxyCertsCMIsValid(ctx context.Context, inj
 
 // EnsureTrustedCAConfigMapExists ensures the trusted CA ConfigMap exists as expected.
 // Creates it if it doesn't exist, patches it if it exists with improper spec.
-func (r *ConfigMapReconciler) EnsureTrustedCAConfigMapExists() error {
-	trustedCA, err := r.k8sclientset.CoreV1().ConfigMaps(r.watchNamespace).Get(context.TODO(),
+func (r *ConfigMapReconciler) EnsureTrustedCAConfigMapExists(ctx context.Context) error {
+	trustedCA, err := r.k8sclientset.CoreV1().ConfigMaps(r.watchNamespace).Get(ctx,
 		certificates.ProxyCertsConfigMap, meta.GetOptions{})
 	if err != nil {
 		if !k8sapierrors.IsNotFound(err) {
 			return err
 		}
-		return r.createProxyCertsCM(context.TODO())
+		return r.createProxyCertsCM(ctx)
 	}
-	return r.ensureProxyCertsCMIsValid(context.TODO(), trustedCA.GetLabels()[InjectionRequestLabel])
+	return r.ensureProxyCertsCMIsValid(ctx, trustedCA.GetLabels()[InjectionRequestLabel])
 }
 
 // EnsureWICDRBAC ensures the WICD RBAC resources exist as expected
@@ -691,8 +691,8 @@ func (r *ConfigMapReconciler) ensureWICDClusterRoleBinding(ctx context.Context)
 // generateServicesManifest generates and regenerates the services manifest.
 // this gets called when the configmap reconciler is first created, to create the services manifest,
 // and also when the rendered-worker configmap is changed, to regenerate it.
-func generateServicesManifest(client client.Client, port string, platform oconfig.PlatformType) (*servicescm.Data, error) {
-	ign, err := ignition.New(client)
+func generateServicesManifest(ctx context.Context, client client.Client, port string, platform oconfig.PlatformType) (*servicescm.Data, error) {
+	ign, err := ignition.New(ctx, client)
 	if err != nil {
 		return nil, fmt.Errorf("error creating ignition object: %w", err)
 	}

controllers/controllerconfig_controller.go

@@ -80,7 +80,7 @@ func (r *ControllerConfigReconciler) Reconcile(ctx context.Context, req ctrl.Req
 		}
 		return ctrl.Result{}, err
 	}
-	r.signer, err = signer.Create(kubeTypes.NamespacedName{Namespace: r.watchNamespace,
+	r.signer, err = signer.Create(ctx, kubeTypes.NamespacedName{Namespace: r.watchNamespace,
 		Name: secrets.PrivateKeySecret}, r.client)
 	if err != nil {
 		return ctrl.Result{}, fmt.Errorf("unable to create signer from private key secret: %w", err)
@@ -93,7 +93,7 @@ func (r *ControllerConfigReconciler) Reconcile(ctx context.Context, req ctrl.Req
 	}
 	// loop Windows nodes and trigger kubelet CA update
 	for _, winNode := range winNodes.Items {
-		if err := r.updateKubeletCA(winNode, cc.Spec.KubeAPIServerServingCAData); err != nil {
+		if err := r.updateKubeletCA(ctx, winNode, cc.Spec.KubeAPIServerServingCAData); err != nil {
 			return ctrl.Result{}, fmt.Errorf("error updating kubelet CA certificate in node %s: %w", winNode.Name, err)
 		}
 	}