From 7c68f83d52c2cb748abcc29e5e4b31ac64d17282 Mon Sep 17 00:00:00 2001 From: Amber Brown Date: Mon, 5 Sep 2022 13:24:14 +1000 Subject: [PATCH] pass in the hive manager directly rather than passing through the restconfig --- pkg/backend/openshiftcluster.go | 29 +++++++++++++++++++++----- pkg/backend/openshiftcluster_test.go | 6 +++--- pkg/cluster/cluster.go | 17 +++++++-------- pkg/cluster/install_version_test.go | 2 +- test/util/testliveconfig/liveconfig.go | 20 ++++++++++++------ 5 files changed, 49 insertions(+), 25 deletions(-) diff --git a/pkg/backend/openshiftcluster.go b/pkg/backend/openshiftcluster.go index 72ff05f01..97be54c69 100644 --- a/pkg/backend/openshiftcluster.go +++ b/pkg/backend/openshiftcluster.go @@ -13,12 +13,12 @@ import ( "github.com/Azure/go-autorest/autorest/azure" "github.com/Azure/go-autorest/autorest/to" "github.com/sirupsen/logrus" - "k8s.io/client-go/rest" "github.com/Azure/ARO-RP/pkg/api" "github.com/Azure/ARO-RP/pkg/cluster" "github.com/Azure/ARO-RP/pkg/database" "github.com/Azure/ARO-RP/pkg/env" + "github.com/Azure/ARO-RP/pkg/hive" "github.com/Azure/ARO-RP/pkg/metrics" "github.com/Azure/ARO-RP/pkg/util/billing" "github.com/Azure/ARO-RP/pkg/util/encryption" @@ -29,7 +29,7 @@ import ( type openShiftClusterBackend struct { *backend - newManager func(context.Context, *logrus.Entry, env.Interface, database.OpenShiftClusters, database.Gateway, database.OpenShiftVersions, encryption.AEAD, billing.Manager, *api.OpenShiftClusterDocument, *api.SubscriptionDocument, *rest.Config, metrics.Emitter) (cluster.Interface, error) + newManager func(context.Context, *logrus.Entry, env.Interface, database.OpenShiftClusters, database.Gateway, database.OpenShiftVersions, encryption.AEAD, billing.Manager, *api.OpenShiftClusterDocument, *api.SubscriptionDocument, hive.ClusterManager, metrics.Emitter) (cluster.Interface, error) } func newOpenShiftClusterBackend(b *backend) *openShiftClusterBackend { @@ -102,12 +102,31 @@ func (ocb *openShiftClusterBackend) handle(ctx context.Context, log *logrus.Entr return err } - hiveRestConfig, err := ocb.env.LiveConfig().HiveRestConfig(ctx, 1) + // Only attempt to access Hive if we are installing via Hive or adopting clusters + installViaHive, err := ocb.env.LiveConfig().InstallViaHive(ctx) if err != nil { - log.Info(err) // TODO(hive): Update to fail once we have Hive everywhere in prod and dev + return err } - m, err := ocb.newManager(ctx, log, ocb.env, ocb.dbOpenShiftClusters, ocb.dbGateway, ocb.dbOpenShiftVersions, ocb.aead, ocb.billing, doc, subscriptionDoc, hiveRestConfig, ocb.m) + adoptViaHive, err := ocb.env.LiveConfig().AdoptByHive(ctx) + if err != nil { + return err + } + + var hr hive.ClusterManager + if installViaHive || adoptViaHive { + hiveShard := 1 + hiveRestConfig, err := ocb.env.LiveConfig().HiveRestConfig(ctx, hiveShard) + if err != nil { + return fmt.Errorf("failed getting RESTConfig for Hive shard %d: %w", hiveShard, err) + } + hr, err = hive.NewFromConfig(log, ocb.env, hiveRestConfig) + if err != nil { + return fmt.Errorf("failed creating HiveClusterManager: %w", err) + } + } + + m, err := ocb.newManager(ctx, log, ocb.env, ocb.dbOpenShiftClusters, ocb.dbGateway, ocb.dbOpenShiftVersions, ocb.aead, ocb.billing, doc, subscriptionDoc, hr, ocb.m) if err != nil { return ocb.endLease(ctx, log, stop, doc, api.ProvisioningStateFailed, err) } diff --git a/pkg/backend/openshiftcluster_test.go b/pkg/backend/openshiftcluster_test.go index e17445662..7387c64ce 100644 --- a/pkg/backend/openshiftcluster_test.go +++ b/pkg/backend/openshiftcluster_test.go @@ -12,12 +12,12 @@ import ( "github.com/golang/mock/gomock" "github.com/sirupsen/logrus" - "k8s.io/client-go/rest" "github.com/Azure/ARO-RP/pkg/api" "github.com/Azure/ARO-RP/pkg/cluster" "github.com/Azure/ARO-RP/pkg/database" "github.com/Azure/ARO-RP/pkg/env" + "github.com/Azure/ARO-RP/pkg/hive" "github.com/Azure/ARO-RP/pkg/metrics" "github.com/Azure/ARO-RP/pkg/metrics/noop" "github.com/Azure/ARO-RP/pkg/util/billing" @@ -281,7 +281,7 @@ func TestBackendTry(t *testing.T) { t.Run(tt.name, func(t *testing.T) { ctx := context.Background() log := logrus.NewEntry(logrus.StandardLogger()) - tlc := testliveconfig.NewTestLiveConfig(false) + tlc := testliveconfig.NewTestLiveConfig(false, false) controller := gomock.NewController(t) defer controller.Finish() @@ -302,7 +302,7 @@ func TestBackendTry(t *testing.T) { t.Fatal(err) } - createManager := func(context.Context, *logrus.Entry, env.Interface, database.OpenShiftClusters, database.Gateway, database.OpenShiftVersions, encryption.AEAD, billing.Manager, *api.OpenShiftClusterDocument, *api.SubscriptionDocument, *rest.Config, metrics.Emitter) (cluster.Interface, error) { + createManager := func(context.Context, *logrus.Entry, env.Interface, database.OpenShiftClusters, database.Gateway, database.OpenShiftVersions, encryption.AEAD, billing.Manager, *api.OpenShiftClusterDocument, *api.SubscriptionDocument, hive.ClusterManager, metrics.Emitter) (cluster.Interface, error) { return manager, nil } diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 5976face8..63a832e0e 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -18,7 +18,6 @@ import ( "github.com/sirupsen/logrus" extensionsclient "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" "k8s.io/client-go/kubernetes" - "k8s.io/client-go/rest" "github.com/Azure/ARO-RP/pkg/api" "github.com/Azure/ARO-RP/pkg/cluster/graph" @@ -100,6 +99,7 @@ type manager struct { imageregistrycli imageregistryclient.Interface installViaHive bool + adoptViaHive bool hiveClusterManager hive.ClusterManager aroOperatorDeployer deploy.Operator @@ -109,7 +109,7 @@ type manager struct { // New returns a cluster manager func New(ctx context.Context, log *logrus.Entry, _env env.Interface, db database.OpenShiftClusters, dbGateway database.Gateway, dbOpenShiftVersions database.OpenShiftVersions, aead encryption.AEAD, - billing billing.Manager, doc *api.OpenShiftClusterDocument, subscriptionDoc *api.SubscriptionDocument, hiveRestConfig *rest.Config, metricsEmitter metrics.Emitter) (Interface, error) { + billing billing.Manager, doc *api.OpenShiftClusterDocument, subscriptionDoc *api.SubscriptionDocument, hiveClusterManager hive.ClusterManager, metricsEmitter metrics.Emitter) (Interface, error) { r, err := azure.ParseResourceID(doc.OpenShiftCluster.ID) if err != nil { return nil, err @@ -137,13 +137,9 @@ func New(ctx context.Context, log *logrus.Entry, _env env.Interface, db database return nil, err } - // TODO(hive): always set hiveClusterManager once we have Hive everywhere in prod and dev - var hr hive.ClusterManager - if hiveRestConfig != nil { - hr, err = hive.NewFromConfig(log, _env, hiveRestConfig) - if err != nil { - return nil, err - } + adoptByHive, err := _env.LiveConfig().AdoptByHive(ctx) + if err != nil { + return nil, err } return &manager{ @@ -182,7 +178,8 @@ func New(ctx context.Context, log *logrus.Entry, _env env.Interface, db database graph: graph.NewManager(log, aead, storage), installViaHive: installViaHive, - hiveClusterManager: hr, + adoptViaHive: adoptByHive, + hiveClusterManager: hiveClusterManager, now: func() time.Time { return time.Now() }, }, nil } diff --git a/pkg/cluster/install_version_test.go b/pkg/cluster/install_version_test.go index 18fb616e8..a39f11516 100644 --- a/pkg/cluster/install_version_test.go +++ b/pkg/cluster/install_version_test.go @@ -95,7 +95,7 @@ func TestGetOpenShiftVersionFromVersion(t *testing.T) { controller := gomock.NewController(t) defer controller.Finish() - tlc := testliveconfig.NewTestLiveConfig(false) + tlc := testliveconfig.NewTestLiveConfig(false, false) _env := mock_env.NewMockInterface(controller) _env.EXPECT().ACRDomain().AnyTimes().Return(testACRDomain) _env.EXPECT().LiveConfig().AnyTimes().Return(tlc) diff --git a/test/util/testliveconfig/liveconfig.go b/test/util/testliveconfig/liveconfig.go index da7498c1c..df9676c0c 100644 --- a/test/util/testliveconfig/liveconfig.go +++ b/test/util/testliveconfig/liveconfig.go @@ -13,27 +13,35 @@ import ( ) type testLiveConfig struct { - hasHive bool + adoptByHive bool + installViaHive bool } func (t *testLiveConfig) HiveRestConfig(ctx context.Context, shard int) (*rest.Config, error) { - if t.hasHive { + if t.adoptByHive || t.installViaHive { return &rest.Config{}, nil } return nil, errors.New("testLiveConfig does not have a Hive") } func (t *testLiveConfig) InstallViaHive(ctx context.Context) (bool, error) { - return t.hasHive, nil + return t.installViaHive, nil +} + +func (t *testLiveConfig) AdoptByHive(ctx context.Context) (bool, error) { + return t.adoptByHive, nil } func (t *testLiveConfig) DefaultInstallerPullSpecOverride(ctx context.Context) string { - if t.hasHive { + if t.installViaHive { return "example/pull:spec" } return "" } -func NewTestLiveConfig(hasHive bool) liveconfig.Manager { - return &testLiveConfig{hasHive: hasHive} +func NewTestLiveConfig(adoptByHive bool, installViaHive bool) liveconfig.Manager { + return &testLiveConfig{ + adoptByHive: adoptByHive, + installViaHive: installViaHive, + } }