Skip to content

Commit

Permalink
Merge pull request #353 from slintes/skip-tno
Browse files Browse the repository at this point in the history
Check control plane topology
  • Loading branch information
openshift-merge-bot[bot] authored Dec 6, 2024
2 parents ef97552 + 2fc8330 commit 6efed12
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 19 deletions.
33 changes: 22 additions & 11 deletions api/v1alpha1/nodehealthcheck_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,35 +33,38 @@ import (
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"

"github.com/medik8s/node-healthcheck-operator/controllers/cluster"
"github.com/medik8s/node-healthcheck-operator/controllers/utils/annotations"
)

const (
OngoingRemediationError = "prohibited due to running remediation"
minHealthyError = "MinHealthy must not be negative"
invalidSelectorError = "Invalid selector"
missingSelectorError = "Selector is mandatory"
mandatoryRemediationError = "Either RemediationTemplate or at least one EscalatingRemediations must be set"
mutualRemediationError = "RemediationTemplate and EscalatingRemediations usage is mutual exclusive"
uniqueOrderError = "EscalatingRemediation Order must be unique"
uniqueRemediatorError = "Using multiple templates of same kind is not supported for this template"
minimumTimeoutError = "EscalatingRemediation Timeout must be at least one minute"
OngoingRemediationError = "prohibited due to running remediation"
minHealthyError = "MinHealthy must not be negative"
invalidSelectorError = "Invalid selector"
missingSelectorError = "Selector is mandatory"
mandatoryRemediationError = "Either RemediationTemplate or at least one EscalatingRemediations must be set"
mutualRemediationError = "RemediationTemplate and EscalatingRemediations usage is mutual exclusive"
uniqueOrderError = "EscalatingRemediation Order must be unique"
uniqueRemediatorError = "Using multiple templates of same kind is not supported for this template"
minimumTimeoutError = "EscalatingRemediation Timeout must be at least one minute"
unsupportedCpTopologyError = "Unsupported control plane topology"
)

// log is for logging in this package.
var nodehealthchecklog = logf.Log.WithName("nodehealthcheck-resource")

func (nhc *NodeHealthCheck) SetupWebhookWithManager(mgr ctrl.Manager) error {
func (nhc *NodeHealthCheck) SetupWebhookWithManager(mgr ctrl.Manager, caps *cluster.Capabilities) error {
return ctrl.NewWebhookManagedBy(mgr).
For(nhc).
WithValidator(&customValidator{mgr.GetClient()}).
WithValidator(&customValidator{mgr.GetClient(), caps}).
Complete()
}

//+kubebuilder:webhook:path=/validate-remediation-medik8s-io-v1alpha1-nodehealthcheck,mutating=false,failurePolicy=fail,sideEffects=None,groups=remediation.medik8s.io,resources=nodehealthchecks,verbs=create;update;delete,versions=v1alpha1,name=vnodehealthcheck.kb.io,admissionReviewVersions=v1

type customValidator struct {
client.Client
caps *cluster.Capabilities
}

// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
Expand Down Expand Up @@ -106,6 +109,7 @@ func (v *customValidator) validate(ctx context.Context, nhc *NodeHealthCheck) er
v.validateSelector(nhc),
v.validateMutualRemediations(nhc),
v.validateEscalatingRemediations(ctx, nhc),
v.validateControlPlaneTopology(),
})

// everything else should have been covered by API server validation
Expand All @@ -114,6 +118,13 @@ func (v *customValidator) validate(ctx context.Context, nhc *NodeHealthCheck) er
return aggregated
}

func (v *customValidator) validateControlPlaneTopology() error {
if !v.caps.IsSupportedControlPlaneTopology {
return fmt.Errorf(unsupportedCpTopologyError)
}
return nil
}

func (v *customValidator) validateMinHealthy(nhc *NodeHealthCheck) error {
// Using Minimum kubebuilder marker for IntOrStr does not work (yet)
if nhc.Spec.MinHealthy == nil {
Expand Down
14 changes: 13 additions & 1 deletion api/v1alpha1/nodehealthcheck_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ var _ = Describe("NodeHealthCheck Validation", func() {
var mockValidatorClient = &mockClient{
listFunc: func(context.Context, client.ObjectList, ...client.ListOption) error { return nil },
}
var validator = &customValidator{mockValidatorClient}
var validator = &customValidator{mockValidatorClient, testCaps}
Context("Creating or updating a NHC", func() {

var nhc *NodeHealthCheck
Expand Down Expand Up @@ -200,6 +200,18 @@ var _ = Describe("NodeHealthCheck Validation", func() {

})
})

Context("with unsupported control plane topology", func() {
BeforeEach(func() {
testCaps.IsSupportedControlPlaneTopology = false
})
AfterEach(func() {
testCaps.IsSupportedControlPlaneTopology = true
})
It("should be denied", func() {
Expect(validator.validate(context.Background(), nhc)).To(MatchError(ContainSubstring(unsupportedCpTopologyError)))
})
})
})

Context("During ongoing remediation", func() {
Expand Down
10 changes: 9 additions & 1 deletion api/v1alpha1/webhook_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ import (
"sigs.k8s.io/controller-runtime/pkg/log/zap"
"sigs.k8s.io/controller-runtime/pkg/metrics/server"
"sigs.k8s.io/controller-runtime/pkg/webhook"

"github.com/medik8s/node-healthcheck-operator/controllers/cluster"
)

// These tests use Ginkgo (BDD-style Go testing framework). Refer to
Expand All @@ -49,6 +51,12 @@ var testEnv *envtest.Environment
var ctx context.Context
var cancel context.CancelFunc

var testCaps = &cluster.Capabilities{
IsOnOpenshift: true,
HasMachineAPI: true,
IsSupportedControlPlaneTopology: true,
}

func TestAPIs(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "Webhook Suite")
Expand Down Expand Up @@ -103,7 +111,7 @@ var _ = BeforeSuite(func() {
})
Expect(err).NotTo(HaveOccurred())

err = (&NodeHealthCheck{}).SetupWebhookWithManager(mgr)
err = (&NodeHealthCheck{}).SetupWebhookWithManager(mgr, testCaps)
Expect(err).NotTo(HaveOccurred())

//+kubebuilder:scaffold:webhook
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,14 @@ spec:
- get
- list
- watch
- apiGroups:
- config.openshift.io
resources:
- infrastructures
verbs:
- get
- list
- watch
- apiGroups:
- console.openshift.io
resources:
Expand Down
8 changes: 8 additions & 0 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ rules:
- get
- list
- watch
- apiGroups:
- config.openshift.io
resources:
- infrastructures
verbs:
- get
- list
- watch
- apiGroups:
- console.openshift.io
resources:
Expand Down
46 changes: 44 additions & 2 deletions controllers/cluster/capabilities.go
Original file line number Diff line number Diff line change
@@ -1,16 +1,29 @@
package cluster

import (
"context"

"github.com/go-logr/logr"

"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/client-go/discovery"
"k8s.io/client-go/rest"
"sigs.k8s.io/controller-runtime/pkg/client"

configv1 "github.com/openshift/api/config/v1"
)

type Capabilities struct {
IsOnOpenshift, HasMachineAPI bool
IsOnOpenshift, HasMachineAPI, IsSupportedControlPlaneTopology bool
}

func NewCapabilities(config *rest.Config) (Capabilities, error) {
//+kubebuilder:rbac:groups=config.openshift.io,resources=infrastructures,verbs=get;list;watch

// NewCapabilities returns a Capabilities struct with the following fields:
// - IsOnOpenshift: true if the cluster is an OpenShift cluster
// - HasMachineAPI: true if the cluster has the Machine API installed
// - IsSupportedControlPlaneTopology: true if the cluster has a supported control plane topology
func NewCapabilities(config *rest.Config, reader client.Reader, logger logr.Logger, ctx context.Context) (Capabilities, error) {
dc, err := discovery.NewDiscoveryClientForConfig(config)
if err != nil {
return Capabilities{}, err
Expand All @@ -25,6 +38,8 @@ func NewCapabilities(config *rest.Config) (Capabilities, error) {
machineAPIGroup := schema.GroupVersion{Group: "machine.openshift.io", Version: "v1"}

caps := Capabilities{}

// check API groups for OCP and MachineAPI groups
for _, apiGroup := range apiGroups.Groups {
for _, supportedVersion := range apiGroup.Versions {
if supportedVersion.GroupVersion == ocpKind.GroupVersion().String() {
Expand All @@ -38,5 +53,32 @@ func NewCapabilities(config *rest.Config) (Capabilities, error) {
}
}
}

// check topology, default to true on non-openshift clusters
caps.IsSupportedControlPlaneTopology = true
if caps.IsOnOpenshift {
infra := &configv1.Infrastructure{}
err = reader.Get(ctx, client.ObjectKey{Name: "cluster"}, infra)
if err != nil {
return Capabilities{}, err
}
cpTopology := infra.Status.ControlPlaneTopology

// add new supported topology modes here if needed
// don't allow new topologies by default, because they might implement their own fencing logic, which can conflict with this operator
// see https://github.com/openshift/api/blob/c1fdeb0788c16659238d93ec45ce2e798c14eb88/config/v1/types_infrastructure.go#L129-L147
if cpTopology == configv1.HighlyAvailableTopologyMode ||
cpTopology == configv1.SingleReplicaTopologyMode ||
cpTopology == configv1.ExternalTopologyMode {

logger.Info("supported control plane topology", "topology", cpTopology)
caps.IsSupportedControlPlaneTopology = true

} else {
logger.Info("unsupported control plane topology", "topology", cpTopology)
caps.IsSupportedControlPlaneTopology = false
}
}

return caps, nil
}
10 changes: 6 additions & 4 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/metrics/server"
"sigs.k8s.io/controller-runtime/pkg/webhook"

configv1 "github.com/openshift/api/config/v1"
consolev1 "github.com/openshift/api/console/v1"
machinev1beta1 "github.com/openshift/api/machine/v1beta1"
operatorv1 "github.com/openshift/api/operator/v1"
Expand Down Expand Up @@ -77,6 +78,7 @@ func init() {
utilruntime.Must(machinev1beta1.Install(scheme))
utilruntime.Must(operatorv1.Install(scheme))
utilruntime.Must(consolev1.Install(scheme))
utilruntime.Must(configv1.Install(scheme))

// +kubebuilder:scaffold:scheme
}
Expand Down Expand Up @@ -137,7 +139,9 @@ func main() {
os.Exit(1)
}

caps, err := cluster.NewCapabilities(mgr.GetConfig())
ctx := ctrl.SetupSignalHandler()

caps, err := cluster.NewCapabilities(mgr.GetConfig(), mgr.GetAPIReader(), setupLog, ctx)
if err != nil {
setupLog.Error(err, "unable to determine cluster capabilities")
os.Exit(1)
Expand Down Expand Up @@ -197,14 +201,12 @@ func main() {
}
}

if err = (&remediationv1alpha1.NodeHealthCheck{}).SetupWebhookWithManager(mgr); err != nil {
if err = (&remediationv1alpha1.NodeHealthCheck{}).SetupWebhookWithManager(mgr, &caps); err != nil {
setupLog.Error(err, "unable to create webhook", "webhook", "NodeHealthCheck")
os.Exit(1)
}
// +kubebuilder:scaffold:builder

ctx := ctrl.SetupSignalHandler()

// Do some initialization
initializer := initializer.New(mgr, caps, ctrl.Log.WithName("Initializer"))
if err = mgr.Add(initializer); err != nil {
Expand Down

0 comments on commit 6efed12

Please sign in to comment.