Merge pull request #309 from medik8s/revert-306-check-machineapi-caps-0

Revert "Ensure MachineAPI is enabled before trying to use its resources"

controllers/cluster/upgrade_checker.go

@@ -13,6 +13,8 @@ import (
 
 	v1 "github.com/openshift/api/config/v1"
 	clusterversion "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1"
+
+	"github.com/medik8s/node-healthcheck-operator/controllers/utils"
 )
 
 var unsupportedUpgradeCheckerErr = errors.New(
@@ -64,8 +66,12 @@ func (n *noopClusterUpgradeStatusChecker) Check() (bool, error) {
 
 // NewClusterUpgradeStatusChecker will return some implementation of a checker or err in case it can't
 // reliably detect which implementation to use.
-func NewClusterUpgradeStatusChecker(mgr manager.Manager, caps Capabilities) (UpgradeChecker, error) {
-	if !caps.IsOnOpenshift {
+func NewClusterUpgradeStatusChecker(mgr manager.Manager) (UpgradeChecker, error) {
+	openshift, err := utils.IsOnOpenshift(mgr.GetConfig())
+	if err != nil {
+		return nil, err
+	}
+	if !openshift {
 		return &noopClusterUpgradeStatusChecker{}, nil
 	}
 	checker, err := newOpenshiftClusterUpgradeChecker(mgr)

controllers/console/plugin.go

@@ -24,11 +24,12 @@ import (
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/rest"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/openshift/api/console/v1alpha1"
 
-	"github.com/medik8s/node-healthcheck-operator/controllers/cluster"
+	"github.com/medik8s/node-healthcheck-operator/controllers/utils"
 )
 
 const (
@@ -46,10 +47,12 @@ const (
 // HEADS UP: consider cleanup of old resources in case of name changes or removals!
 //
 // TODO image reference to console plugin in CSV?
-func CreateOrUpdatePlugin(ctx context.Context, cl client.Client, caps cluster.Capabilities, namespace string, log logr.Logger) error {
+func CreateOrUpdatePlugin(ctx context.Context, cl client.Client, config *rest.Config, namespace string, log logr.Logger) error {
 
 	// check if we are on OCP
-	if !caps.IsOnOpenshift {
+	if isOpenshift, err := utils.IsOnOpenshift(config); err != nil {
+		return errors.Wrap(err, "failed to check if we are on Openshift")
+	} else if !isOpenshift {
 		log.Info("we are not on Openshift, skipping console plugin activation")
 		return nil
 	}

controllers/initializer/init.go

@@ -6,10 +6,10 @@ import (
 	"github.com/go-logr/logr"
 	"github.com/pkg/errors"
 
+	"k8s.io/client-go/rest"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
-	"github.com/medik8s/node-healthcheck-operator/controllers/cluster"
 	"github.com/medik8s/node-healthcheck-operator/controllers/console"
 	"github.com/medik8s/node-healthcheck-operator/controllers/rbac"
 	"github.com/medik8s/node-healthcheck-operator/controllers/utils"
@@ -20,17 +20,17 @@ import (
 // - create default NHC
 // - create console plugin
 type initializer struct {
-	cl           client.Client
-	capabilities cluster.Capabilities
-	logger       logr.Logger
+	cl     client.Client
+	config *rest.Config
+	logger logr.Logger
 }
 
 // New returns a new Initializer
-func New(mgr ctrl.Manager, caps cluster.Capabilities, logger logr.Logger) *initializer {
+func New(mgr ctrl.Manager, logger logr.Logger) *initializer {
 	return &initializer{
-		cl:           mgr.GetClient(),
-		capabilities: caps,
-		logger:       logger,
+		cl:     mgr.GetClient(),
+		config: mgr.GetConfig(),
+		logger: logger,
 	}
 }
 
@@ -46,7 +46,7 @@ func (i *initializer) Start(ctx context.Context) error {
 		return errors.Wrap(err, "failed to create or update RBAC aggregation role")
 	}
 
-	if err = console.CreateOrUpdatePlugin(ctx, i.cl, i.capabilities, ns, ctrl.Log.WithName("console-plugin")); err != nil {
+	if err = console.CreateOrUpdatePlugin(ctx, i.cl, i.config, ns, ctrl.Log.WithName("console-plugin")); err != nil {
 		return errors.Wrap(err, "failed to create or update the console plugin")
 	}
 

controllers/initializer/init_test.go

@@ -7,17 +7,14 @@ import (
 	. "github.com/onsi/gomega"
 
 	ctrl "sigs.k8s.io/controller-runtime"
-
-	"github.com/medik8s/node-healthcheck-operator/controllers/cluster"
 )
 
 var _ = Describe("Init", func() {
 
 	var initializer *initializer
 
 	JustBeforeEach(func() {
-		caps := cluster.Capabilities{IsOnOpenshift: true, HasMachineAPI: true}
-		initializer = New(k8sManager, caps, ctrl.Log.WithName("test initializer"))
+		initializer = New(k8sManager, ctrl.Log.WithName("test initializer"))
 	})
 
 	AfterEach(func() {

controllers/initializer/suite_test.go

@@ -36,8 +36,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/metrics/server"
 
-	consolev1alpha "github.com/openshift/api/console/v1alpha1"
-
 	remediationv1alpha1 "github.com/medik8s/node-healthcheck-operator/api/v1alpha1"
 )
 
@@ -65,13 +63,7 @@ var _ = BeforeSuite(func() {
 
 	By("bootstrapping test environment")
 	testEnv = &envtest.Environment{
-		CRDInstallOptions: envtest.CRDInstallOptions{
-			Paths: []string{
-				filepath.Join("..", "..", "vendor", "github.com", "openshift", "api", "console", "v1alpha1"),
-				filepath.Join("..", "..", "config", "crd", "bases"),
-			},
-			ErrorIfPathMissing: true,
-		},
+		CRDDirectoryPaths: []string{filepath.Join("..", "..", "config", "crd", "bases")},
 	}
 
 	var err error
@@ -80,8 +72,8 @@ var _ = BeforeSuite(func() {
 	Expect(cfg).NotTo(BeNil())
 
 	scheme.AddToScheme(scheme.Scheme)
-	Expect(remediationv1alpha1.AddToScheme(scheme.Scheme)).NotTo(HaveOccurred())
-	Expect(consolev1alpha.AddToScheme(scheme.Scheme)).NotTo(HaveOccurred())
+	err = remediationv1alpha1.AddToScheme(scheme.Scheme)
+	Expect(err).NotTo(HaveOccurred())
 
 	k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme})
 	Expect(err).NotTo(HaveOccurred())

controllers/machinehealthcheck_controller_test.go

@@ -33,7 +33,6 @@ import (
 
 	machinev1 "github.com/openshift/api/machine/v1beta1"
 
-	"github.com/medik8s/node-healthcheck-operator/controllers/cluster"
 	"github.com/medik8s/node-healthcheck-operator/controllers/featuregates"
 	"github.com/medik8s/node-healthcheck-operator/controllers/mhc"
 	"github.com/medik8s/node-healthcheck-operator/controllers/resources"
@@ -2388,8 +2387,7 @@ func newFakeReconcilerWithCustomRecorder(recorder record.EventRecorder, initObje
 		WithRuntimeObjects(initObjects...).
 		WithStatusSubresource(&machinev1.MachineHealthCheck{}).
 		Build()
-	caps := cluster.Capabilities{IsOnOpenshift: false, HasMachineAPI: false}
-	mhcChecker, _ := mhc.NewMHCChecker(k8sManager, caps, nil)
+	mhcChecker, _ := mhc.NewMHCChecker(k8sManager, false, nil)
 	return &MachineHealthCheckReconciler{
 		Client:                         fakeClient,
 		Recorder:                       recorder,

controllers/mhc/checker.go

@@ -11,8 +11,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 
 	"github.com/openshift/api/machine/v1beta1"
-
-	"github.com/medik8s/node-healthcheck-operator/controllers/cluster"
 )
 
 // NodeConditionTerminating is the node condition type used by the termination handler MHC
@@ -27,9 +25,9 @@ type Checker interface {
 }
 
 // NewMHCChecker creates a new Checker
-func NewMHCChecker(mgr manager.Manager, caps cluster.Capabilities, mhcEvents chan<- event.GenericEvent) (Checker, error) {
+func NewMHCChecker(mgr manager.Manager, onOpenshift bool, mhcEvents chan<- event.GenericEvent) (Checker, error) {
 
-	if !caps.HasMachineAPI {
+	if !onOpenshift {
 		return DummyChecker{}, nil
 	}
 

controllers/nodehealthcheck_controller.go

@@ -85,7 +85,7 @@ type NodeHealthCheckReconciler struct {
 	Recorder                    record.EventRecorder
 	ClusterUpgradeStatusChecker cluster.UpgradeChecker
 	MHCChecker                  mhc.Checker
-	Capabilities                cluster.Capabilities
+	OnOpenShift                 bool
 	MHCEvents                   chan event.GenericEvent
 	controller                  controller.Controller
 	watches                     map[string]struct{}
@@ -163,7 +163,7 @@ func (r *NodeHealthCheckReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 	if err != nil {
 		return result, err
 	}
-	resourceManager := resources.NewManager(r.Client, ctx, r.Log, r.Capabilities.HasMachineAPI, leaseManager, r.Recorder)
+	resourceManager := resources.NewManager(r.Client, ctx, r.Log, r.OnOpenShift, leaseManager, r.Recorder)
 
 	// always check if we need to patch status before we exit Reconcile
 	nhcOrig := nhc.DeepCopy()
@@ -693,7 +693,7 @@ func (r *NodeHealthCheckReconciler) isControlPlaneRemediationAllowed(ctx context
 	}
 
 	// no ongoing control plane remediation, check etcd quorum
-	if !r.Capabilities.IsOnOpenshift {
+	if !r.OnOpenShift {
 		// etcd quorum PDB is only installed in OpenShift
 		return true, nil
 	}

controllers/resources/manager.go

@@ -51,23 +51,23 @@ func (r RemediationCRNotOwned) Error() string { return r.msg }
 
 type manager struct {
 	client.Client
-	ctx           context.Context
-	log           logr.Logger
-	hasMachineAPI bool
-	leaseManager  LeaseManager
-	recorder      record.EventRecorder
+	ctx          context.Context
+	log          logr.Logger
+	onOpenshift  bool
+	leaseManager LeaseManager
+	recorder     record.EventRecorder
 }
 
 var _ Manager = &manager{}
 
-func NewManager(c client.Client, ctx context.Context, log logr.Logger, hasMachineAPI bool, leaseManager LeaseManager, recorder record.EventRecorder) Manager {
+func NewManager(c client.Client, ctx context.Context, log logr.Logger, onOpenshift bool, leaseManager LeaseManager, recorder record.EventRecorder) Manager {
 	return &manager{
-		Client:        c,
-		ctx:           ctx,
-		log:           log.WithName("resource manager"),
-		hasMachineAPI: hasMachineAPI,
-		leaseManager:  leaseManager,
-		recorder:      recorder,
+		Client:       c,
+		ctx:          ctx,
+		log:          log.WithName("resource manager"),
+		onOpenshift:  onOpenshift,
+		leaseManager: leaseManager,
+		recorder:     recorder,
 	}
 }
 
@@ -78,7 +78,7 @@ func (m *manager) GenerateRemediationCRForNode(node *corev1.Node, owner client.O
 	// also set the node's machine as owner ref if possible
 	// TODO also handle CAPI clusters / machines
 	var machineOwnerRef *metav1.OwnerReference
-	if m.hasMachineAPI {
+	if m.onOpenshift {
 		ref, machineNamespace, err := m.getOwningMachineWithNamespace(node)
 		if err != nil {
 			return nil, err

controllers/suite_test.go

@@ -196,9 +196,7 @@ var _ = BeforeSuite(func() {
 		Upgrading: false,
 	}
 
-	caps := cluster.Capabilities{IsOnOpenshift: true, HasMachineAPI: true}
-
-	mhcChecker, err := mhc.NewMHCChecker(k8sManager, caps, nil)
+	mhcChecker, err := mhc.NewMHCChecker(k8sManager, false, nil)
 	Expect(err).NotTo(HaveOccurred())
 
 	os.Setenv("DEPLOYMENT_NAMESPACE", DeploymentNamespace)
@@ -226,7 +224,7 @@ var _ = BeforeSuite(func() {
 		ClusterUpgradeStatusChecker: upgradeChecker,
 		MHCChecker:                  mhcChecker,
 		MHCEvents:                   mhcEvents,
-		Capabilities:                caps,
+		OnOpenShift:                 true,
 	}).SetupWithManager(k8sManager)
 	Expect(err).NotTo(HaveOccurred())
 

controllers/utils/utils.go

@@ -11,6 +11,9 @@ import (
 
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/client-go/discovery"
+	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/cache"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -42,6 +45,24 @@ func GetDeploymentNamespace() (string, error) {
 	return ns, nil
 }
 
+// IsOnOpenshift returns true if the cluster has the openshift config group
+func IsOnOpenshift(config *rest.Config) (bool, error) {
+	dc, err := discovery.NewDiscoveryClientForConfig(config)
+	if err != nil {
+		return false, err
+	}
+	apiGroups, err := dc.ServerGroups()
+	kind := schema.GroupVersionKind{Group: "config.openshift.io", Version: "v1", Kind: "ClusterVersion"}
+	for _, apiGroup := range apiGroups.Groups {
+		for _, supportedVersion := range apiGroup.Versions {
+			if supportedVersion.GroupVersion == kind.GroupVersion().String() {
+				return true, nil
+			}
+		}
+	}
+	return false, nil
+}
+
 // GetLogWithNHC return a logger with NHC namespace and name
 func GetLogWithNHC(log logr.Logger, nhc *v1alpha1.NodeHealthCheck) logr.Logger {
 	return log.WithValues("NodeHealthCheck name", nhc.Name)