Skip to content

Commit df79b73

Browse files
clobranoclobrano
authored
Merge pull request #306 from clobrano/check-machineapi-caps-0
Ensure MachineAPI is enabled before trying to use its resources
2 parents 1133f71 + a3be256 commit df79b73

File tree

13 files changed

+146
-78
lines changed

13 files changed

+146
-78
lines changed

‎controllers/cluster/capabilities.go‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
package cluster
2+
3+
import (
4+
"context"
5+
6+
"github.com/pkg/errors"
7+
8+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
9+
"k8s.io/apimachinery/pkg/runtime/schema"
10+
"k8s.io/client-go/discovery"
11+
"k8s.io/client-go/rest"
12+
13+
v1 "github.com/openshift/api/config/v1"
14+
configv1 "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1"
15+
)
16+
17+
type Capabilities struct {
18+
IsOnOpenshift, HasMachineAPI bool
19+
}
20+
21+
func NewCapabilities(config *rest.Config) (Capabilities, error) {
22+
var err error
23+
var onOpenshift, machineAPI bool
24+
25+
if onOpenshift, err = isOnOpenshift(config); err != nil {
26+
return Capabilities{}, errors.Wrap(err, "failed to check if cluster is on OpenShift")
27+
} else if onOpenshift {
28+
if machineAPI, err = hasMachineAPICapability(config); err != nil {
29+
return Capabilities{}, errors.Wrap(err, "failed to check machine API capability")
30+
}
31+
}
32+
33+
return Capabilities{
34+
IsOnOpenshift: onOpenshift,
35+
HasMachineAPI: machineAPI,
36+
}, nil
37+
}
38+
39+
// IsOnOpenshift returns true if the cluster has the openshift config group
40+
func isOnOpenshift(config *rest.Config) (bool, error) {
41+
dc, err := discovery.NewDiscoveryClientForConfig(config)
42+
if err != nil {
43+
return false, err
44+
}
45+
apiGroups, err := dc.ServerGroups()
46+
kind := schema.GroupVersionKind{Group: "config.openshift.io", Version: "v1", Kind: "ClusterVersion"}
47+
for _, apiGroup := range apiGroups.Groups {
48+
for _, supportedVersion := range apiGroup.Versions {
49+
if supportedVersion.GroupVersion == kind.GroupVersion().String() {
50+
return true, nil
51+
}
52+
}
53+
}
54+
return false, nil
55+
}
56+
57+
// HasMachineAPICapability returns true if the cluster has the MachineAPI Enabled
58+
func hasMachineAPICapability(config *rest.Config) (bool, error) {
59+
configV1Client, err := configv1.NewForConfig(config)
60+
if err != nil {
61+
return false, errors.Wrapf(err, "failed to create cluster version client")
62+
}
63+
cvs, err := configV1Client.ClusterVersions().List(context.Background(), metav1.ListOptions{})
64+
if err != nil {
65+
return false, errors.Wrapf(err, "failed to get ClusterVersion")
66+
}
67+
68+
for _, cv := range cvs.Items {
69+
if cv.Status.Capabilities.EnabledCapabilities == nil {
70+
return false, nil
71+
}
72+
var MachineAPI v1.ClusterVersionCapability = "MachineAPI"
73+
for _, capability := range cv.Status.Capabilities.EnabledCapabilities {
74+
if capability == MachineAPI {
75+
return true, nil
76+
}
77+
}
78+
}
79+
return false, nil
80+
}

‎controllers/cluster/upgrade_checker.go‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@ import (
1313

1414
v1 "github.com/openshift/api/config/v1"
1515
clusterversion "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1"
16-
17-
"github.com/medik8s/node-healthcheck-operator/controllers/utils"
1816
)
1917

2018
var unsupportedUpgradeCheckerErr = errors.New(
@@ -66,12 +64,8 @@ func (n *noopClusterUpgradeStatusChecker) Check() (bool, error) {
6664

6765
// NewClusterUpgradeStatusChecker will return some implementation of a checker or err in case it can't
6866
// reliably detect which implementation to use.
69-
func NewClusterUpgradeStatusChecker(mgr manager.Manager) (UpgradeChecker, error) {
70-
openshift, err := utils.IsOnOpenshift(mgr.GetConfig())
71-
if err != nil {
72-
return nil, err
73-
}
74-
if !openshift {
67+
func NewClusterUpgradeStatusChecker(mgr manager.Manager, caps Capabilities) (UpgradeChecker, error) {
68+
if !caps.IsOnOpenshift {
7569
return &noopClusterUpgradeStatusChecker{}, nil
7670
}
7771
checker, err := newOpenshiftClusterUpgradeChecker(mgr)

‎controllers/console/plugin.go‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,11 @@ import (
2424

2525
apierrors "k8s.io/apimachinery/pkg/api/errors"
2626
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
27-
"k8s.io/client-go/rest"
2827
"sigs.k8s.io/controller-runtime/pkg/client"
2928

3029
"github.com/openshift/api/console/v1alpha1"
3130

32-
"github.com/medik8s/node-healthcheck-operator/controllers/utils"
31+
"github.com/medik8s/node-healthcheck-operator/controllers/cluster"
3332
)
3433

3534
const (
@@ -47,12 +46,10 @@ const (
4746
// HEADS UP: consider cleanup of old resources in case of name changes or removals!
4847
//
4948
// TODO image reference to console plugin in CSV?
50-
func CreateOrUpdatePlugin(ctx context.Context, cl client.Client, config *rest.Config, namespace string, log logr.Logger) error {
49+
func CreateOrUpdatePlugin(ctx context.Context, cl client.Client, caps cluster.Capabilities, namespace string, log logr.Logger) error {
5150

5251
// check if we are on OCP
53-
if isOpenshift, err := utils.IsOnOpenshift(config); err != nil {
54-
return errors.Wrap(err, "failed to check if we are on Openshift")
55-
} else if !isOpenshift {
52+
if !caps.IsOnOpenshift {
5653
log.Info("we are not on Openshift, skipping console plugin activation")
5754
return nil
5855
}

‎controllers/initializer/init.go‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@ import (
66
"github.com/go-logr/logr"
77
"github.com/pkg/errors"
88

9-
"k8s.io/client-go/rest"
109
ctrl "sigs.k8s.io/controller-runtime"
1110
"sigs.k8s.io/controller-runtime/pkg/client"
1211

12+
"github.com/medik8s/node-healthcheck-operator/controllers/cluster"
1313
"github.com/medik8s/node-healthcheck-operator/controllers/console"
1414
"github.com/medik8s/node-healthcheck-operator/controllers/rbac"
1515
"github.com/medik8s/node-healthcheck-operator/controllers/utils"
@@ -20,17 +20,17 @@ import (
2020
// - create default NHC
2121
// - create console plugin
2222
type initializer struct {
23-
cl client.Client
24-
config *rest.Config
25-
logger logr.Logger
23+
cl client.Client
24+
capabilities cluster.Capabilities
25+
logger logr.Logger
2626
}
2727

2828
// New returns a new Initializer
29-
func New(mgr ctrl.Manager, logger logr.Logger) *initializer {
29+
func New(mgr ctrl.Manager, caps cluster.Capabilities, logger logr.Logger) *initializer {
3030
return &initializer{
31-
cl: mgr.GetClient(),
32-
config: mgr.GetConfig(),
33-
logger: logger,
31+
cl: mgr.GetClient(),
32+
capabilities: caps,
33+
logger: logger,
3434
}
3535
}
3636

@@ -46,7 +46,7 @@ func (i *initializer) Start(ctx context.Context) error {
4646
return errors.Wrap(err, "failed to create or update RBAC aggregation role")
4747
}
4848

49-
if err = console.CreateOrUpdatePlugin(ctx, i.cl, i.config, ns, ctrl.Log.WithName("console-plugin")); err != nil {
49+
if err = console.CreateOrUpdatePlugin(ctx, i.cl, i.capabilities, ns, ctrl.Log.WithName("console-plugin")); err != nil {
5050
return errors.Wrap(err, "failed to create or update the console plugin")
5151
}
5252

‎controllers/initializer/init_test.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,17 @@ import (
77
. "github.com/onsi/gomega"
88

99
ctrl "sigs.k8s.io/controller-runtime"
10+
11+
"github.com/medik8s/node-healthcheck-operator/controllers/cluster"
1012
)
1113

1214
var _ = Describe("Init", func() {
1315

1416
var initializer *initializer
1517

1618
JustBeforeEach(func() {
17-
initializer = New(k8sManager, ctrl.Log.WithName("test initializer"))
19+
caps := cluster.Capabilities{IsOnOpenshift: true, HasMachineAPI: true}
20+
initializer = New(k8sManager, caps, ctrl.Log.WithName("test initializer"))
1821
})
1922

2023
AfterEach(func() {

‎controllers/initializer/suite_test.go‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ import (
3636
"sigs.k8s.io/controller-runtime/pkg/manager"
3737
"sigs.k8s.io/controller-runtime/pkg/metrics/server"
3838

39+
consolev1alpha "github.com/openshift/api/console/v1alpha1"
40+
3941
remediationv1alpha1 "github.com/medik8s/node-healthcheck-operator/api/v1alpha1"
4042
)
4143

@@ -63,7 +65,13 @@ var _ = BeforeSuite(func() {
6365

6466
By("bootstrapping test environment")
6567
testEnv = &envtest.Environment{
66-
CRDDirectoryPaths: []string{filepath.Join("..", "..", "config", "crd", "bases")},
68+
CRDInstallOptions: envtest.CRDInstallOptions{
69+
Paths: []string{
70+
filepath.Join("..", "..", "vendor", "github.com", "openshift", "api", "console", "v1alpha1"),
71+
filepath.Join("..", "..", "config", "crd", "bases"),
72+
},
73+
ErrorIfPathMissing: true,
74+
},
6775
}
6876

6977
var err error
@@ -72,8 +80,8 @@ var _ = BeforeSuite(func() {
7280
Expect(cfg).NotTo(BeNil())
7381

7482
scheme.AddToScheme(scheme.Scheme)
75-
err = remediationv1alpha1.AddToScheme(scheme.Scheme)
76-
Expect(err).NotTo(HaveOccurred())
83+
Expect(remediationv1alpha1.AddToScheme(scheme.Scheme)).NotTo(HaveOccurred())
84+
Expect(consolev1alpha.AddToScheme(scheme.Scheme)).NotTo(HaveOccurred())
7785

7886
k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme})
7987
Expect(err).NotTo(HaveOccurred())

‎controllers/machinehealthcheck_controller_test.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333

3434
machinev1 "github.com/openshift/api/machine/v1beta1"
3535

36+
"github.com/medik8s/node-healthcheck-operator/controllers/cluster"
3637
"github.com/medik8s/node-healthcheck-operator/controllers/featuregates"
3738
"github.com/medik8s/node-healthcheck-operator/controllers/mhc"
3839
"github.com/medik8s/node-healthcheck-operator/controllers/resources"
@@ -2387,7 +2388,8 @@ func newFakeReconcilerWithCustomRecorder(recorder record.EventRecorder, initObje
23872388
WithRuntimeObjects(initObjects...).
23882389
WithStatusSubresource(&machinev1.MachineHealthCheck{}).
23892390
Build()
2390-
mhcChecker, _ := mhc.NewMHCChecker(k8sManager, false, nil)
2391+
caps := cluster.Capabilities{IsOnOpenshift: false, HasMachineAPI: false}
2392+
mhcChecker, _ := mhc.NewMHCChecker(k8sManager, caps, nil)
23912393
return &MachineHealthCheckReconciler{
23922394
Client: fakeClient,
23932395
Recorder: recorder,

‎controllers/mhc/checker.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import (
1111
"sigs.k8s.io/controller-runtime/pkg/manager"
1212

1313
"github.com/openshift/api/machine/v1beta1"
14+
15+
"github.com/medik8s/node-healthcheck-operator/controllers/cluster"
1416
)
1517

1618
// NodeConditionTerminating is the node condition type used by the termination handler MHC
@@ -25,9 +27,9 @@ type Checker interface {
2527
}
2628

2729
// NewMHCChecker creates a new Checker
28-
func NewMHCChecker(mgr manager.Manager, onOpenshift bool, mhcEvents chan<- event.GenericEvent) (Checker, error) {
30+
func NewMHCChecker(mgr manager.Manager, caps cluster.Capabilities, mhcEvents chan<- event.GenericEvent) (Checker, error) {
2931

30-
if !onOpenshift {
32+
if !caps.HasMachineAPI {
3133
return DummyChecker{}, nil
3234
}
3335

‎controllers/nodehealthcheck_controller.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ type NodeHealthCheckReconciler struct {
8585
Recorder record.EventRecorder
8686
ClusterUpgradeStatusChecker cluster.UpgradeChecker
8787
MHCChecker mhc.Checker
88-
OnOpenShift bool
88+
Capabilities cluster.Capabilities
8989
MHCEvents chan event.GenericEvent
9090
controller controller.Controller
9191
watches map[string]struct{}
@@ -163,7 +163,7 @@ func (r *NodeHealthCheckReconciler) Reconcile(ctx context.Context, req ctrl.Requ
163163
if err != nil {
164164
return result, err
165165
}
166-
resourceManager := resources.NewManager(r.Client, ctx, r.Log, r.OnOpenShift, leaseManager, r.Recorder)
166+
resourceManager := resources.NewManager(r.Client, ctx, r.Log, r.Capabilities.HasMachineAPI, leaseManager, r.Recorder)
167167

168168
// always check if we need to patch status before we exit Reconcile
169169
nhcOrig := nhc.DeepCopy()
@@ -693,7 +693,7 @@ func (r *NodeHealthCheckReconciler) isControlPlaneRemediationAllowed(ctx context
693693
}
694694

695695
// no ongoing control plane remediation, check etcd quorum
696-
if !r.OnOpenShift {
696+
if !r.Capabilities.IsOnOpenshift {
697697
// etcd quorum PDB is only installed in OpenShift
698698
return true, nil
699699
}

‎controllers/resources/manager.go‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -51,23 +51,23 @@ func (r RemediationCRNotOwned) Error() string { return r.msg }
5151

5252
type manager struct {
5353
client.Client
54-
ctx context.Context
55-
log logr.Logger
56-
onOpenshift bool
57-
leaseManager LeaseManager
58-
recorder record.EventRecorder
54+
ctx context.Context
55+
log logr.Logger
56+
hasMachineAPI bool
57+
leaseManager LeaseManager
58+
recorder record.EventRecorder
5959
}
6060

6161
var _ Manager = &manager{}
6262

63-
func NewManager(c client.Client, ctx context.Context, log logr.Logger, onOpenshift bool, leaseManager LeaseManager, recorder record.EventRecorder) Manager {
63+
func NewManager(c client.Client, ctx context.Context, log logr.Logger, hasMachineAPI bool, leaseManager LeaseManager, recorder record.EventRecorder) Manager {
6464
return &manager{
65-
Client: c,
66-
ctx: ctx,
67-
log: log.WithName("resource manager"),
68-
onOpenshift: onOpenshift,
69-
leaseManager: leaseManager,
70-
recorder: recorder,
65+
Client: c,
66+
ctx: ctx,
67+
log: log.WithName("resource manager"),
68+
hasMachineAPI: hasMachineAPI,
69+
leaseManager: leaseManager,
70+
recorder: recorder,
7171
}
7272
}
7373

@@ -78,7 +78,7 @@ func (m *manager) GenerateRemediationCRForNode(node *corev1.Node, owner client.O
7878
// also set the node's machine as owner ref if possible
7979
// TODO also handle CAPI clusters / machines
8080
var machineOwnerRef *metav1.OwnerReference
81-
if m.onOpenshift {
81+
if m.hasMachineAPI {
8282
ref, machineNamespace, err := m.getOwningMachineWithNamespace(node)
8383
if err != nil {
8484
return nil, err

0 commit comments

Comments
 (0)