Merge pull request #352 from kostola/MTSRE-1044_3

Add ocmClusterID parameter to package operator reconciler
openshift · May 4, 2023 · fec5510 · fec5510
2 parents 8fcac2d + 48f66b7
commit fec5510
Show file tree

Hide file tree

Showing 7 changed files with 56 additions and 16 deletions.
diff --git a/integration/pko_test.go b/integration/pko_test.go
@@ -32,10 +32,10 @@ const (
 	deadMansSnitchUrlValue = "https://example.com/test-snitch-url"
 	pagerDutyKeyValue      = "1234567890ABCDEF"
 
-	// source: https://github.com/kostola/package-operator-packages/tree/v2.0/openshift/addon-operator/apnp-test-optional-params
-	pkoImageOptionalParams = "quay.io/alcosta/package-operator-packages/openshift/addon-operator/apnp-test-optional-params:v2.0"
-	// source: https://github.com/kostola/package-operator-packages/tree/v2.0/openshift/addon-operator/apnp-test-required-params
-	pkoImageRequiredParams = "quay.io/alcosta/package-operator-packages/openshift/addon-operator/apnp-test-required-params:v2.0"
+	// source: https://github.com/kostola/package-operator-packages/tree/v3.0/openshift/addon-operator/apnp-test-optional-params
+	pkoImageOptionalParams = "quay.io/alcosta/package-operator-packages/openshift/addon-operator/apnp-test-optional-params:v3.0"
+	// source: https://github.com/kostola/package-operator-packages/tree/v3.0/openshift/addon-operator/apnp-test-required-params
+	pkoImageRequiredParams = "quay.io/alcosta/package-operator-packages/openshift/addon-operator/apnp-test-required-params:v3.0"
 )
 
 func (s *integrationTestSuite) TestPackageOperatorReconcilerStatusPropagatedToAddon() {
@@ -355,6 +355,9 @@ func clusterPackageChecker(
 			clusterIDValueOk = err == nil
 		}
 
+		ocmClusterID, present := addonsv1[addon.OcmClusterIDConfigKey]
+		ocmClusterIDValueOk := present && len(fmt.Sprintf("%v", ocmClusterID)) > 0
+
 		addonParametersValueOk, deadMansSnitchUrlValueOk, pagerDutyValueOk := false, false, false
 		if addonParametersValuePresent {
 			value, present := addonsv1[addon.ParametersConfigKey]
@@ -383,15 +386,17 @@ func clusterPackageChecker(
 			pagerDutyValueOk = !present
 		}
 
-		logger.Info(fmt.Sprintf("targetNamespace=%t, clusterID=%t, addonParameters=%t, deadMansSnitchUrl=%t, pagerDutyKey=%t",
+		logger.Info(fmt.Sprintf("targetNamespace=%t, clusterID=%t, ocmClusterID=%t, addonParameters=%t, deadMansSnitchUrl=%t, pagerDutyKey=%t",
 			targetNamespaceValueOk,
 			clusterIDValueOk,
+			ocmClusterIDValueOk,
 			addonParametersValueOk,
 			deadMansSnitchUrlValueOk,
 			pagerDutyValueOk))
 
 		result := targetNamespaceValueOk &&
 			clusterIDValueOk &&
+			ocmClusterIDValueOk &&
 			addonParametersValueOk &&
 			deadMansSnitchUrlValueOk &&
 			pagerDutyValueOk

diff --git a/internal/controllers/addon/addon_reconciler_options.go b/internal/controllers/addon/addon_reconciler_options.go
@@ -37,9 +37,10 @@ type WithPackageOperatorReconciler struct {
 
 func (w WithPackageOperatorReconciler) ApplyToAddonReconciler(config *AddonReconciler) {
 	poReconciler := &PackageOperatorReconciler{
-		Client:    w.Client,
-		Scheme:    w.Scheme,
-		ClusterID: config.ClusterExternalID,
+		Client:       w.Client,
+		Scheme:       w.Scheme,
+		ClusterID:    config.ClusterExternalID,
+		OcmClusterID: config.GetOCMClusterID,
 	}
 	config.subReconcilers = append(config.subReconcilers, poReconciler)
 }

diff --git a/internal/controllers/addon/controller.go b/internal/controllers/addon/controller.go
@@ -129,6 +129,7 @@ func NewAddonReconciler(
 }
 
 type ocmClient interface {
+	GetClusterID() string
 	GetCluster(
 		ctx context.Context,
 		req ocm.ClusterGetRequest,
@@ -173,6 +174,16 @@ func (r *AddonReconciler) InjectOCMClient(ctx context.Context, c *ocm.Client) er
 	return nil
 }
 
+func (r *AddonReconciler) GetOCMClusterID() string {
+	r.ocmClientMux.RLock()
+	defer r.ocmClientMux.RUnlock()
+
+	if r.ocmClient == nil {
+		return ""
+	}
+	return r.ocmClient.GetClusterID()
+}
+
 // Pauses reconcilation of all Addon objects. Concurrency safe.
 func (r *AddonReconciler) EnableGlobalPause(ctx context.Context) error {
 	return r.setGlobalPause(ctx, true)
@@ -248,8 +259,8 @@ func (r *AddonReconciler) SetupWithManager(mgr ctrl.Manager, opts ...AddonReconc
 
 // AddonReconciler/Controller entrypoint
 func (r *AddonReconciler) Reconcile(
-	ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
-
+	ctx context.Context, req ctrl.Request,
+) (ctrl.Result, error) {
 	logger := r.Log.WithValues("addon", req.NamespacedName.String())
 	ctx = controllers.ContextWithLogger(ctx, logger)
 
@@ -298,7 +309,8 @@ func (r *AddonReconciler) syncWithExternalAPIs(ctx context.Context, logger logr.
 }
 
 func (r *AddonReconciler) reconcile(ctx context.Context, addon *addonsv1alpha1.Addon,
-	log logr.Logger) (ctrl.Result, error) {
+	log logr.Logger,
+) (ctrl.Result, error) {
 	ctx = controllers.ContextWithLogger(ctx, log)
 	// Handle addon deletion before checking for pause condition.
 	// This allows even paused addons to be deleted.

diff --git a/internal/controllers/addon/package_operator_reconciler.go b/internal/controllers/addon/package_operator_reconciler.go
@@ -30,23 +30,27 @@ spec:
 		merge
 			(.config | b64decMap)
 			(hasKey .config "%[4]s" | ternary (dict "%[4]s" (index .config "%[4]s" | b64decMap)) (dict))
-			(dict "%[5]s" "%[6]s" "%[7]s" "%[8]s")
+			(dict "%[5]s" "%[6]s" "%[7]s" "%[8]s" "%[9]s" "%[10]s")
 	)}}
 `
 
 const (
 	packageOperatorName        = "packageOperatorReconciler"
 	ClusterIDConfigKey         = "clusterID"
 	DeadMansSnitchUrlConfigKey = "deadMansSnitchUrl"
+	OcmClusterIDConfigKey      = "ocmClusterID"
 	PagerDutyKeyConfigKey      = "pagerDutyKey"
 	ParametersConfigKey        = "parameters"
 	TargetNamespaceConfigKey   = "targetNamespace"
 )
 
+type OcmClusterIDGetter func() string
+
 type PackageOperatorReconciler struct {
-	Client    client.Client
-	Scheme    *runtime.Scheme
-	ClusterID string
+	Client       client.Client
+	Scheme       *runtime.Scheme
+	ClusterID    string
+	OcmClusterID OcmClusterIDGetter
 }
 
 func (r *PackageOperatorReconciler) Name() string { return packageOperatorName }
@@ -71,6 +75,7 @@ func (r *PackageOperatorReconciler) reconcileClusterObjectTemplate(ctx context.C
 		addon.Spec.AddonPackageOperator.Image,
 		ParametersConfigKey,
 		ClusterIDConfigKey, r.ClusterID,
+		OcmClusterIDConfigKey, r.OcmClusterID(),
 		TargetNamespaceConfigKey, addonDestNamespace,
 	)
 

diff --git a/internal/controllers/addon/package_operator_reconciler_test.go b/internal/controllers/addon/package_operator_reconciler_test.go
@@ -142,7 +142,14 @@ func TestPackageOperatorReconcilerLogic(t *testing.T) {
 					ctx := context.Background()
 					subTest.configureClient(ctx, c, identifier)
 
-					r := &addon.PackageOperatorReconciler{Client: c, Scheme: testutil.NewTestSchemeWithAddonsv1alpha1()}
+					r := &addon.PackageOperatorReconciler{
+						Client:    c,
+						Scheme:    testutil.NewTestSchemeWithAddonsv1alpha1(),
+						ClusterID: "test-cluster-id",
+						OcmClusterID: func() string {
+							return "test-ocm-cluster-id"
+						},
+					}
 
 					res, err := r.Reconcile(ctx, test.addon)
 

diff --git a/internal/ocm/cluster.go b/internal/ocm/cluster.go
@@ -18,6 +18,10 @@ type Cluster struct {
 	ExternalId string `json:"external_id"`
 }
 
+func (c *Client) GetClusterID() string {
+	return c.opts.ClusterID
+}
+
 func (c *Client) GetCluster(
 	ctx context.Context,
 	req ClusterGetRequest,

diff --git a/internal/ocm/ocmtest/mock.go b/internal/ocm/ocmtest/mock.go
@@ -8,6 +8,8 @@ import (
 	"github.com/openshift/addon-operator/internal/ocm"
 )
 
+const MockClusterID = "foobar"
+
 type Client struct {
 	mock.Mock
 }
@@ -43,6 +45,10 @@ func (c *Client) GetCluster(
 		args.Error(1)
 }
 
+func (c *Client) GetClusterID() string {
+	return MockClusterID
+}
+
 func (c *Client) PostAddOnStatus(
 	ctx context.Context,
 	req ocm.AddOnStatusPostRequest,