Update unittests and scheme

Rewrite unittests to accommodate changes in controller-runtime v0.15.0 and create a hive specific scheme. One such major change involves the behavior of the fake client deletionTimestamp. The fake client will panic if initialized with an object that has a DeletionTimestamp and no finalizer, and will silently ignore a DeletionTimestamp on an object in a Create request, matching the behavior of the kube-apiserver. Tests have been rewritten to work with this new behavior. kubernetes-sigs/controller-runtime#2316 This update also includes a requirement to register a fake client WithStatusSubresource in order to successfully update the Status subresource of an object. Due to this change, the helper function NewFakeClientBuilder() is added to pkg/test/fake to allow for easier setup of a fake client with the necessary sub-resources. kubernetes-sigs/controller-runtime#2259 Lastly, a new singleton scheme has been introduced to the codebase. The hive specific scheme is registered once, with all necessary packages, and used throughout the codebase, removing the need to specify unique schemes for each use case, and decreasing the overall amount of imports. Signed-off-by: Leah Leshchinsky <lleshchi@redhat.com>
lleshchi · Aug 14, 2023 · d5694f7 · d5694f7
1 parent 9b793ba
commit d5694f7
Show file tree

Hide file tree

Showing 80 changed files with 608 additions and 816 deletions.
diff --git a/cmd/hiveadmission/main.go b/cmd/hiveadmission/main.go
@@ -4,10 +4,9 @@ import (
 	admissionCmd "github.com/openshift/generic-admission-server/pkg/cmd"
 	log "github.com/sirupsen/logrus"
 
-	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
-	hivev1 "github.com/openshift/hive/apis/hive/v1"
+	"github.com/openshift/hive/pkg/util/scheme"
 	hivevalidatingwebhooks "github.com/openshift/hive/pkg/validating-webhooks/hive/v1"
 	"github.com/openshift/hive/pkg/version"
 )
@@ -35,8 +34,7 @@ func main() {
 }
 
 func createDecoder() *admission.Decoder {
-	scheme := runtime.NewScheme()
-	hivev1.AddToScheme(scheme)
+	scheme := scheme.GetScheme()
 	decoder := admission.NewDecoder(scheme)
 	// FIXME check for error?
 	return decoder

diff --git a/cmd/manager/main.go b/cmd/manager/main.go
@@ -10,25 +10,20 @@ import (
 	"os"
 	"time"
 
-	velerov1 "github.com/heptio/velero/pkg/apis/velero/v1"
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 
-	apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/wait"
 	_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
-	crv1alpha1 "k8s.io/cluster-registry/pkg/apis/clusterregistry/v1alpha1"
 	"k8s.io/klog"
 	"sigs.k8s.io/controller-runtime/pkg/client/config"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/manager/signals"
 
-	openshiftapiv1 "github.com/openshift/api/config/v1"
 	_ "github.com/openshift/generic-admission-server/pkg/cmd"
 
-	"github.com/openshift/hive/apis"
 	hivev1 "github.com/openshift/hive/apis/hive/v1"
 	cmdutil "github.com/openshift/hive/cmd/util"
 	"github.com/openshift/hive/pkg/constants"
@@ -57,6 +52,7 @@ import (
 	"github.com/openshift/hive/pkg/controller/utils"
 	"github.com/openshift/hive/pkg/controller/velerobackup"
 	utillogrus "github.com/openshift/hive/pkg/util/logrus"
+	"github.com/openshift/hive/pkg/util/scheme"
 	"github.com/openshift/hive/pkg/version"
 )
 
@@ -159,6 +155,7 @@ func newRootCommand() *cobra.Command {
 			run := func(ctx context.Context) {
 				// Create a new Cmd to provide shared dependencies and start components
 				mgr, err := manager.New(cfg, manager.Options{
+					Scheme:             scheme.GetScheme(),
 					MetricsBindAddress: ":2112",
 					Logger:             utillogrus.NewLogr(log.StandardLogger()),
 				})
@@ -172,27 +169,6 @@ func newRootCommand() *cobra.Command {
 					log.Fatal(err)
 				}
 
-				// Setup Scheme for all resources
-				if err := apis.AddToScheme(mgr.GetScheme()); err != nil {
-					log.Fatal(err)
-				}
-
-				if err := openshiftapiv1.Install(mgr.GetScheme()); err != nil {
-					log.Fatal(err)
-				}
-
-				if err := apiextv1.AddToScheme(mgr.GetScheme()); err != nil {
-					log.Fatal(err)
-				}
-
-				if err := crv1alpha1.AddToScheme(mgr.GetScheme()); err != nil {
-					log.Fatal(err)
-				}
-
-				if err := velerov1.AddToScheme(mgr.GetScheme()); err != nil {
-					log.Fatal(err)
-				}
-
 				disabledControllersSet := sets.NewString(opts.DisabledControllers...)
 				// Setup all Controllers
 				for _, name := range opts.Controllers {

diff --git a/cmd/operator/main.go b/cmd/operator/main.go
@@ -12,25 +12,20 @@ import (
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 
-	apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apimachinery/pkg/util/wait"
 	_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
 	"k8s.io/klog"
-	apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client/config"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/manager/signals"
 
-	oappsv1 "github.com/openshift/api/apps/v1"
-	orbacv1 "github.com/openshift/api/authorization/v1"
-	oconfigv1 "github.com/openshift/api/config/v1"
 	_ "github.com/openshift/generic-admission-server/pkg/cmd"
 
-	"github.com/openshift/hive/apis"
 	cmdutil "github.com/openshift/hive/cmd/util"
 	"github.com/openshift/hive/pkg/operator"
 	"github.com/openshift/hive/pkg/operator/hive"
 	utillogrus "github.com/openshift/hive/pkg/util/logrus"
+	"github.com/openshift/hive/pkg/util/scheme"
 	"github.com/openshift/hive/pkg/version"
 )
 
@@ -88,6 +83,7 @@ func newRootCommand() *cobra.Command {
 			run := func(ctx context.Context) {
 				// Create a new Cmd to provide shared dependencies and start components
 				mgr, err := manager.New(cfg, manager.Options{
+					Scheme:             scheme.GetScheme(),
 					MetricsBindAddress: ":2112",
 					Logger:             utillogrus.NewLogr(log.StandardLogger()),
 				})
@@ -97,31 +93,6 @@ func newRootCommand() *cobra.Command {
 
 				log.Info("Registering Components.")
 
-				// Setup Scheme for all resources
-				if err := apis.AddToScheme(mgr.GetScheme()); err != nil {
-					log.Fatal(err)
-				}
-
-				if err := apiregistrationv1.AddToScheme(mgr.GetScheme()); err != nil {
-					log.Fatal(err)
-				}
-
-				if err := apiextv1.AddToScheme(mgr.GetScheme()); err != nil {
-					log.Fatal(err)
-				}
-
-				if err := oappsv1.Install(mgr.GetScheme()); err != nil {
-					log.Fatal(err)
-				}
-
-				if err := orbacv1.Install(mgr.GetScheme()); err != nil {
-					log.Fatal(err)
-				}
-
-				if err := oconfigv1.Install(mgr.GetScheme()); err != nil {
-					log.Fatal(err)
-				}
-
 				// Setup all Controllers
 				if err := operator.AddToOperator(mgr); err != nil {
 					log.Fatal(err)

diff --git a/contrib/pkg/adm/managedns/enable.go b/contrib/pkg/adm/managedns/enable.go
@@ -23,11 +23,9 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/client-go/kubernetes/scheme"
 
 	"sigs.k8s.io/controller-runtime/pkg/client/config"
 
-	"github.com/openshift/hive/apis"
 	hivev1 "github.com/openshift/hive/apis/hive/v1"
 	hiveutils "github.com/openshift/hive/contrib/pkg/utils"
 	awsutils "github.com/openshift/hive/contrib/pkg/utils/aws"
@@ -36,6 +34,7 @@ import (
 	hiveclient "github.com/openshift/hive/pkg/client/clientset/versioned"
 	"github.com/openshift/hive/pkg/constants"
 	"github.com/openshift/hive/pkg/resource"
+	"github.com/openshift/hive/pkg/util/scheme"
 )
 
 const longDesc = `
@@ -121,9 +120,7 @@ func (o *Options) Validate(cmd *cobra.Command) error {
 
 // Run executes the command
 func (o *Options) Run(args []string) error {
-	if err := apis.AddToScheme(scheme.Scheme); err != nil {
-		return err
-	}
+	scheme := scheme.GetScheme()
 	rh, err := o.getResourceHelper()
 	if err != nil {
 		return err
@@ -189,7 +186,7 @@ func (o *Options) Run(args []string) error {
 
 	log.Infof("created cloud credentials secret: %s", credsSecret.Name)
 	credsSecret.Namespace = hiveNSName
-	if _, err := rh.ApplyRuntimeObject(credsSecret, scheme.Scheme); err != nil {
+	if _, err := rh.ApplyRuntimeObject(credsSecret, scheme); err != nil {
 		log.WithError(err).Fatal("failed to save generated secret")
 	}
 

diff --git a/contrib/pkg/clusterpool/clusterclaim.go b/contrib/pkg/clusterpool/clusterclaim.go
@@ -9,11 +9,10 @@ import (
 	"github.com/spf13/cobra"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
 
-	"github.com/openshift/hive/apis"
 	hivev1 "github.com/openshift/hive/apis/hive/v1"
 	"github.com/openshift/hive/contrib/pkg/utils"
+	"github.com/openshift/hive/pkg/util/scheme"
 )
 
 type ClusterClaimOptions struct {
@@ -52,10 +51,7 @@ func NewClaimClusterPoolCommand() *cobra.Command {
 }
 
 func (o ClusterClaimOptions) run() error {
-	scheme := runtime.NewScheme()
-	if err := apis.AddToScheme(scheme); err != nil {
-		return err
-	}
+	scheme := scheme.GetScheme()
 	claim := o.generateClaim()
 
 	rh, err := utils.GetResourceHelper(o.log)

diff --git a/contrib/pkg/clusterpool/clusterpool.go b/contrib/pkg/clusterpool/clusterpool.go
@@ -18,13 +18,13 @@ import (
 	"k8s.io/cli-runtime/pkg/printers"
 	"k8s.io/client-go/util/homedir"
 
-	"github.com/openshift/hive/apis"
 	hivev1 "github.com/openshift/hive/apis/hive/v1"
 	"github.com/openshift/hive/contrib/pkg/utils"
 	awsutils "github.com/openshift/hive/contrib/pkg/utils/aws"
 	azureutils "github.com/openshift/hive/contrib/pkg/utils/azure"
 	gcputils "github.com/openshift/hive/contrib/pkg/utils/gcp"
 	"github.com/openshift/hive/pkg/clusterresource"
+	"github.com/openshift/hive/pkg/util/scheme"
 )
 
 const (
@@ -213,10 +213,7 @@ func (o *ClusterPoolOptions) validate(cmd *cobra.Command) error {
 
 // run executes the command
 func (o *ClusterPoolOptions) run() error {
-	scheme := runtime.NewScheme()
-	if err := apis.AddToScheme(scheme); err != nil {
-		return err
-	}
+	scheme := scheme.GetScheme()
 
 	objs, err := o.generateObjects()
 	if err != nil {

diff --git a/contrib/pkg/createcluster/create.go b/contrib/pkg/createcluster/create.go
@@ -18,9 +18,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/cli-runtime/pkg/printers"
-	"k8s.io/client-go/kubernetes/scheme"
 
-	"github.com/openshift/hive/apis"
 	hivev1 "github.com/openshift/hive/apis/hive/v1"
 	hivev1azure "github.com/openshift/hive/apis/hive/v1/azure"
 	"github.com/openshift/hive/contrib/pkg/utils"
@@ -33,6 +31,7 @@ import (
 	"github.com/openshift/hive/pkg/clusterresource"
 	"github.com/openshift/hive/pkg/constants"
 	"github.com/openshift/hive/pkg/gcpclient"
+	"github.com/openshift/hive/pkg/util/scheme"
 	installertypes "github.com/openshift/installer/pkg/types"
 	"github.com/openshift/installer/pkg/validate"
 )
@@ -497,9 +496,7 @@ func (o *Options) Validate(cmd *cobra.Command) error {
 
 // Run executes the command
 func (o *Options) Run() error {
-	if err := apis.AddToScheme(scheme.Scheme); err != nil {
-		return err
-	}
+	scheme := scheme.GetScheme()
 
 	objs, err := o.GenerateObjects()
 	if err != nil {
@@ -512,7 +509,7 @@ func (o *Options) Run() error {
 		} else {
 			printer = &printers.JSONPrinter{}
 		}
-		printObjects(objs, scheme.Scheme, printer)
+		printObjects(objs, scheme, printer)
 		return err
 	}
 	rh, err := utils.GetResourceHelper(o.log)
@@ -533,7 +530,7 @@ func (o *Options) Run() error {
 			return err
 		}
 		accessor.SetNamespace(o.Namespace)
-		if _, err := rh.ApplyRuntimeObject(obj, scheme.Scheme); err != nil {
+		if _, err := rh.ApplyRuntimeObject(obj, scheme); err != nil {
 			return err
 		}
 

diff --git a/contrib/pkg/report/deprovisioning.go b/contrib/pkg/report/deprovisioning.go
@@ -8,11 +8,9 @@ import (
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 
-	"github.com/openshift/hive/apis"
 	hivev1 "github.com/openshift/hive/apis/hive/v1"
 	contributils "github.com/openshift/hive/contrib/pkg/utils"
 
-	"k8s.io/client-go/kubernetes/scheme"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -67,10 +65,6 @@ func (o *DeprovisioningReportOptions) Validate(cmd *cobra.Command) error {
 
 // Run executes the command
 func (o *DeprovisioningReportOptions) Run(dynClient client.Client) error {
-	if err := apis.AddToScheme(scheme.Scheme); err != nil {
-		return err
-	}
-
 	cdList := &hivev1.ClusterDeploymentList{}
 	err := dynClient.List(context.Background(), cdList)
 	if err != nil {

diff --git a/contrib/pkg/report/provisioning.go b/contrib/pkg/report/provisioning.go
@@ -8,14 +8,12 @@ import (
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 
-	"github.com/openshift/hive/apis"
 	hivev1 "github.com/openshift/hive/apis/hive/v1"
 	contributils "github.com/openshift/hive/contrib/pkg/utils"
 
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
-	"k8s.io/client-go/kubernetes/scheme"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -76,10 +74,6 @@ func (o *ProvisioningReportOptions) Validate(cmd *cobra.Command) error {
 
 // Run executes the command
 func (o *ProvisioningReportOptions) Run(dynClient client.Client) error {
-	if err := apis.AddToScheme(scheme.Scheme); err != nil {
-		return err
-	}
-
 	var ageLT, ageGT *time.Duration
 	var err error
 	if o.AgeLT != "" {

diff --git a/contrib/pkg/utils/client.go b/contrib/pkg/utils/client.go
@@ -1,12 +1,10 @@
 package utils
 
 import (
-	"k8s.io/client-go/kubernetes/scheme"
+	"github.com/openshift/hive/pkg/util/scheme"
 	restclient "k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
 
-	"github.com/openshift/hive/apis"
-
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -17,8 +15,7 @@ func GetClient() (client.Client, error) {
 		return nil, err
 	}
 
-	apis.AddToScheme(scheme.Scheme)
-	dynamicClient, err := client.New(cfg, client.Options{})
+	dynamicClient, err := client.New(cfg, client.Options{Scheme: scheme.GetScheme()})
 	if err != nil {
 		return nil, err
 	}

diff --git a/pkg/clusterresource/builder_test.go b/pkg/clusterresource/builder_test.go
@@ -6,14 +6,12 @@ import (
 	"testing"
 
 	"github.com/ghodss/yaml"
-	"github.com/openshift/hive/apis"
 	hivev1 "github.com/openshift/hive/apis/hive/v1"
 	hivev1azure "github.com/openshift/hive/apis/hive/v1/azure"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/client-go/kubernetes/scheme"
 )
 
 const (
@@ -312,7 +310,6 @@ metadata:
 	}
 
 	for _, test := range tests {
-		apis.AddToScheme(scheme.Scheme)
 		t.Run(test.name, func(t *testing.T) {
 			require.NoError(t, test.builder.Validate())
 			allObjects, err := test.builder.Build()