Scorecard finalizer fix (#2597)

* remove hardcoded wait time in scorecard cleanup, instead using the initTimeout configuration setting

* make linter happy by shortening a line

* update changelog, update scorecard docs, replace hard coded timeout value with config init-timeout value

Author: Jeff McCormick

CHANGELOG.md

@@ -21,6 +21,7 @@
     - The prepare/converge/verify tasks now make use of the new `k8s` `wait` option to simplify the deployment logic.
 - Operator user setup and entrypoint scripts no longer insert dynamic runtime user entries into `/etc/passwd`. To use dynamic runtime users, use a container runtime that supports it (e.g. CRI-O). ([#2469](https://github.com/operator-framework/operator-sdk/pull/2469))
 - Changed the scorecard basic test, `Writing into CRs has an effect`, to include the http.MethodPatch as part of its test criteria alongside http.MethodPut and http.MethodPost. ([#2509](https://github.com/operator-framework/operator-sdk/pull/2509))
+- Changed the scorecard to use the init-timeout configuration setting as a wait time when performing cleanup instead of a hard-coded time.  ([#2597](https://github.com/operator-framework/operator-sdk/pull/2597))
 
 ### Deprecated
 

doc/test-framework/scorecard.md

@@ -125,7 +125,7 @@ The `basic` and `olm` internal plugins have the same configuration fields:
 | `olm-deployed` | bool | indicates that the CSV and relevant CRD's have been deployed onto the cluster by the [Operator Lifecycle Manager (OLM)][olm] |
 | `kubeconfig` | string | path to kubeconfig. If both the global `kubeconfig` and this field are set, this field is used for the plugin |
 | `namespace` | string | namespace to run the plugins in. If not set, the default specified by the kubeconfig is used |
-| `init-timeout` | int | time in seconds until a timeout during initialization of the operator |
+| `init-timeout` | int | time in seconds until a timeout during initialization or cleanup of the operator |
 | `crds-dir` | string | path to directory containing CRDs that must be deployed to the cluster |
 | `namespaced-manifest` | string | manifest file with all resources that run within a namespace. By default, the scorecard will combine `service_account.yaml`, `role.yaml`, `role_binding.yaml`, and `operator.yaml` from the `deploy` directory into a temporary manifest to use as the namespaced manifest |
 | `global-manifest` | string | manifest containing required resources that run globally (not namespaced). By default, the scorecard will combine all CRDs in the `crds-dir` directory into a temporary manifest to use as the global manifest |

internal/scorecard/plugins/plugin_runner.go

@@ -392,35 +392,35 @@ func duplicateCRCheck(crs []string) error {
 	return nil
 }
 
-func runTests(csv *olmapiv1alpha1.ClusterServiceVersion, pluginType PluginType, config BasicAndOLMPluginConfig,
+func runTests(csv *olmapiv1alpha1.ClusterServiceVersion, pluginType PluginType, cfg BasicAndOLMPluginConfig,
 	cr string, logFile io.Writer) ([]schelpers.TestResult, string, error) {
 	testResults := make([]schelpers.TestResult, 0)
 
 	logReadWriter := &bytes.Buffer{}
 	log.SetOutput(logReadWriter)
 	log.Printf("Running for cr: %s", cr)
 
-	if !config.OLMDeployed {
-		if err := createFromYAMLFile(config.Namespace, config.GlobalManifest, config.ProxyImage,
-			config.ProxyPullPolicy); err != nil {
+	if !cfg.OLMDeployed {
+		if err := createFromYAMLFile(cfg.Namespace, cfg.GlobalManifest, cfg.ProxyImage,
+			cfg.ProxyPullPolicy, cfg.InitTimeout); err != nil {
 			return testResults, "", fmt.Errorf("failed to create global resources: %v", err)
 		}
-		if err := createFromYAMLFile(config.Namespace, config.NamespacedManifest, config.ProxyImage,
-			config.ProxyPullPolicy); err != nil {
+		if err := createFromYAMLFile(cfg.Namespace, cfg.NamespacedManifest, cfg.ProxyImage,
+			cfg.ProxyPullPolicy, cfg.InitTimeout); err != nil {
 			return testResults, "", fmt.Errorf("failed to create namespaced resources: %v", err)
 		}
 	}
 
-	if err := createFromYAMLFile(config.Namespace, cr, config.ProxyImage, config.ProxyPullPolicy); err != nil {
+	if err := createFromYAMLFile(cfg.Namespace, cr, cfg.ProxyImage, cfg.ProxyPullPolicy, cfg.InitTimeout); err != nil {
 		return testResults, "", fmt.Errorf("failed to create cr resource: %v", err)
 	}
 
-	obj, err := yamlToUnstructured(config.Namespace, cr)
+	obj, err := yamlToUnstructured(cfg.Namespace, cr)
 	if err != nil {
 		return testResults, "", fmt.Errorf("failed to decode custom resource manifest into object: %s", err)
 	}
 
-	if err := waitUntilCRStatusExists(time.Second*time.Duration(config.InitTimeout), obj); err != nil {
+	if err := waitUntilCRStatusExists(time.Second*time.Duration(cfg.InitTimeout), obj); err != nil {
 		return testResults, "", fmt.Errorf("failed waiting to check if CR status exists: %v", err)
 	}
 
@@ -443,9 +443,9 @@ func runTests(csv *olmapiv1alpha1.ClusterServiceVersion, pluginType PluginType,
 			Client:   runtimeClient,
 			CR:       obj,
 			CSV:      csv,
-			CRDsDir:  config.CRDsDir,
+			CRDsDir:  cfg.CRDsDir,
 			ProxyPod: proxyPodGlobal,
-			Bundle:   config.Bundle,
+			Bundle:   cfg.Bundle,
 		}
 
 		tests = append(tests, NewBundleValidationTest(conf))
@@ -456,7 +456,7 @@ func runTests(csv *olmapiv1alpha1.ClusterServiceVersion, pluginType PluginType,
 
 	}
 
-	tests = applySelector(tests, config.Selector)
+	tests = applySelector(tests, cfg.Selector)
 
 	for _, test := range tests {
 		testResults = append(testResults, *test.Run(context.TODO()))

internal/scorecard/plugins/resource_handler.go

@@ -87,7 +87,7 @@ func yamlToUnstructured(namespace, yamlPath string) (*unstructured.Unstructured,
 
 // createFromYAMLFile will take a path to a YAML file and create the resource. If it finds a
 // deployment, it will add the scorecard proxy as a container in the deployments podspec.
-func createFromYAMLFile(namespace, yamlPath, proxyImage string, pullPolicy v1.PullPolicy) error {
+func createFromYAMLFile(namespace, yamlPath, proxyImage string, pullPolicy v1.PullPolicy, initTimeout int) error {
 	yamlSpecs, err := ioutil.ReadFile(yamlPath)
 	if err != nil {
 		return fmt.Errorf("failed to read file %s: %v", yamlPath, err)
@@ -115,7 +115,7 @@ func createFromYAMLFile(namespace, yamlPath, proxyImage string, pullPolicy v1.Pu
 				return fmt.Errorf("failed to convert object to deployment: %v", err)
 			}
 			deploymentName = dep.GetName()
-			err = createKubeconfigSecret(namespace)
+			err = createKubeconfigSecret(namespace, initTimeout)
 			if err != nil {
 				return fmt.Errorf("failed to create kubeconfig secret for scorecard-proxy: %v", err)
 			}
@@ -148,7 +148,7 @@ func createFromYAMLFile(namespace, yamlPath, proxyImage string, pullPolicy v1.Pu
 				return err
 			}
 		}
-		addResourceCleanup(obj, types.NamespacedName{Namespace: obj.GetNamespace(), Name: obj.GetName()})
+		addResourceCleanup(obj, types.NamespacedName{Namespace: obj.GetNamespace(), Name: obj.GetName()}, initTimeout)
 		if obj.GetKind() == "Deployment" {
 			proxyPodGlobal, err = getPodFromDeployment(deploymentName, namespace)
 			if err != nil {
@@ -216,7 +216,7 @@ func getPodFromDeployment(depName, namespace string) (pod *v1.Pod, err error) {
 
 // createKubeconfigSecret creates the secret that will be mounted in the operator's container and contains
 // the kubeconfig for communicating with the proxy
-func createKubeconfigSecret(namespace string) error {
+func createKubeconfigSecret(namespace string, initTimeout int) error {
 	kubeconfigMap := make(map[string][]byte)
 	kc, err := proxyConf.Create(metav1.OwnerReference{Name: "scorecard"}, "http://localhost:8889", namespace)
 	if err != nil {
@@ -248,7 +248,7 @@ func createKubeconfigSecret(namespace string) error {
 		return err
 	}
 	addResourceCleanup(kubeconfigSecret, types.NamespacedName{Namespace: kubeconfigSecret.GetNamespace(),
-		Name: kubeconfigSecret.GetName()})
+		Name: kubeconfigSecret.GetName()}, initTimeout)
 	return nil
 }
 
@@ -345,15 +345,15 @@ func cleanupScorecard() error {
 }
 
 // addResourceCleanup adds a cleanup function for the specified runtime object
-func addResourceCleanup(obj runtime.Object, key types.NamespacedName) {
+func addResourceCleanup(obj runtime.Object, key types.NamespacedName, initTimeout int) {
 	cleanupFns = append(cleanupFns, func() error {
 		// make a copy of the object because the client changes it
 		objCopy := obj.DeepCopyObject()
 		err := runtimeClient.Delete(context.TODO(), obj)
 		if err != nil && !apierrors.IsNotFound(err) {
 			return err
 		}
-		err = wait.PollImmediate(time.Second*1, time.Second*10, func() (bool, error) {
+		err = wait.PollImmediate(time.Second*1, time.Second*time.Duration(initTimeout), func() (bool, error) {
 			err = runtimeClient.Get(context.TODO(), key, objCopy)
 			if err != nil {
 				if apierrors.IsNotFound(err) {