Skip to content

Commit 1a2c56d

Browse files
committed
MAO to stop managing metal3 deployments if CBO is available
In an upgrade scenario, check if CBO is available to take ownership of the metal3 deployment. In that case, back off and let CBO manage it. In a downgrade/rollback scenario where the metal3 deployment is annotated as being owned by cluster-baremetal-operator, but the baremetal operator has been removed, take back control of the metal3 deployment.
1 parent 23447c1 commit 1a2c56d

File tree

4 files changed

+211
-1
lines changed

4 files changed

+211
-1
lines changed

pkg/operator/baremetal_pod.go

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,18 @@ import (
99
"golang.org/x/crypto/bcrypt"
1010

1111
"github.com/golang/glog"
12+
osclientset "github.com/openshift/client-go/config/clientset/versioned"
1213
appsv1 "k8s.io/api/apps/v1"
1314
corev1 "k8s.io/api/core/v1"
1415
apierrors "k8s.io/apimachinery/pkg/api/errors"
1516
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
17+
appsclientv1 "k8s.io/client-go/kubernetes/typed/apps/v1"
1618
coreclientv1 "k8s.io/client-go/kubernetes/typed/core/v1"
1719
"k8s.io/utils/pointer"
1820
)
1921

2022
const (
23+
baremetalDeploymentName = "metal3"
2124
baremetalSharedVolume = "metal3-shared"
2225
baremetalSecretName = "metal3-mariadb-password"
2326
baremetalSecretKey = "password"
@@ -234,13 +237,40 @@ func createMetal3PasswordSecrets(client coreclientv1.SecretsGetter, config *Oper
234237
return nil
235238
}
236239

240+
// Return false on error or if "baremetal.openshift.io/owned" annotation set
241+
func checkMetal3DeploymentMAOOwned(client appsclientv1.DeploymentsGetter, config *OperatorConfig) (bool, error) {
242+
existing, err := client.Deployments(config.TargetNamespace).Get(context.Background(), baremetalDeploymentName, metav1.GetOptions{})
243+
if err != nil {
244+
if apierrors.IsNotFound(err) {
245+
return true, nil
246+
}
247+
return false, err
248+
}
249+
if _, exists := existing.ObjectMeta.Annotations[cboOwnedAnnotation]; exists {
250+
return false, nil
251+
}
252+
return true, nil
253+
}
254+
255+
// Return true if the baremetal clusteroperator exists
256+
func checkForBaremetalClusterOperator(osClient osclientset.Interface) (bool, error) {
257+
_, err := osClient.ConfigV1().ClusterOperators().Get(context.Background(), cboClusterOperatorName, metav1.GetOptions{})
258+
if err != nil {
259+
if apierrors.IsNotFound(err) {
260+
return false, nil
261+
}
262+
return false, err
263+
}
264+
return true, nil
265+
}
266+
237267
func newMetal3Deployment(config *OperatorConfig, baremetalProvisioningConfig BaremetalProvisioningConfig) *appsv1.Deployment {
238268
replicas := int32(1)
239269
template := newMetal3PodTemplateSpec(config, baremetalProvisioningConfig)
240270

241271
return &appsv1.Deployment{
242272
ObjectMeta: metav1.ObjectMeta{
243-
Name: "metal3",
273+
Name: baremetalDeploymentName,
244274
Namespace: config.TargetNamespace,
245275
Annotations: map[string]string{
246276
maoOwnedAnnotation: "",

pkg/operator/baremetal_test.go

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@ import (
44
"testing"
55

66
. "github.com/onsi/gomega"
7+
osconfigv1 "github.com/openshift/api/config/v1"
8+
fakeos "github.com/openshift/client-go/config/clientset/versioned/fake"
79
"golang.org/x/net/context"
10+
appsv1 "k8s.io/api/apps/v1"
811
apierrors "k8s.io/apimachinery/pkg/api/errors"
912
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1013
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -375,7 +378,163 @@ func TestSyncBaremetalControllers(t *testing.T) {
375378

376379
err := optr.syncBaremetalControllers(operatorConfig, tc.configCRName)
377380
g.Expect(err).To(Equal(tc.expectedError))
381+
})
382+
}
383+
}
384+
385+
func TestCheckMetal3DeploymentOwned(t *testing.T) {
386+
kubeClient := fakekube.NewSimpleClientset(nil...)
387+
operatorConfig := newOperatorWithBaremetalConfig()
388+
client := kubeClient.AppsV1()
378389

390+
testCases := []struct {
391+
testCase string
392+
deployment *appsv1.Deployment
393+
expected bool
394+
expectedError bool
395+
}{
396+
{
397+
testCase: "Only maoOwnedAnnotation",
398+
deployment: &appsv1.Deployment{
399+
TypeMeta: metav1.TypeMeta{
400+
Kind: "Deployment",
401+
APIVersion: "apps/v1",
402+
},
403+
ObjectMeta: metav1.ObjectMeta{
404+
Name: baremetalDeploymentName,
405+
Annotations: map[string]string{
406+
maoOwnedAnnotation: "",
407+
},
408+
},
409+
},
410+
expected: true,
411+
},
412+
{
413+
testCase: "Only cboOwnedAnnotation",
414+
deployment: &appsv1.Deployment{
415+
TypeMeta: metav1.TypeMeta{
416+
Kind: "Deployment",
417+
APIVersion: "apps/v1",
418+
},
419+
ObjectMeta: metav1.ObjectMeta{
420+
Name: baremetalDeploymentName,
421+
Annotations: map[string]string{
422+
cboOwnedAnnotation: "",
423+
},
424+
},
425+
},
426+
expected: false,
427+
},
428+
{
429+
testCase: "Both cboOwnedAnnotation and maoOwnedAnnotation",
430+
deployment: &appsv1.Deployment{
431+
TypeMeta: metav1.TypeMeta{
432+
Kind: "Deployment",
433+
APIVersion: "apps/v1",
434+
},
435+
ObjectMeta: metav1.ObjectMeta{
436+
Name: baremetalDeploymentName,
437+
Annotations: map[string]string{
438+
cboOwnedAnnotation: "",
439+
maoOwnedAnnotation: "",
440+
},
441+
},
442+
},
443+
expected: false,
444+
},
445+
{
446+
testCase: "No cboOwnedAnnotation or maoOwnedAnnotation",
447+
deployment: &appsv1.Deployment{
448+
TypeMeta: metav1.TypeMeta{
449+
Kind: "Deployment",
450+
APIVersion: "apps/v1",
451+
},
452+
ObjectMeta: metav1.ObjectMeta{
453+
Name: baremetalDeploymentName,
454+
Annotations: map[string]string{},
455+
},
456+
},
457+
expected: true,
458+
},
459+
}
460+
for _, tc := range testCases {
461+
t.Run(string(tc.testCase), func(t *testing.T) {
462+
463+
_, err := client.Deployments("test-namespace").Create(context.Background(), tc.deployment, metav1.CreateOptions{})
464+
if err != nil {
465+
t.Fatalf("Could not create metal3 test deployment.\n")
466+
}
467+
maoOwned, err := checkMetal3DeploymentMAOOwned(client, operatorConfig)
468+
if maoOwned != tc.expected {
469+
t.Errorf("Expected: %v, got: %v", tc.expected, maoOwned)
470+
}
471+
if tc.expectedError != (err != nil) {
472+
t.Errorf("ExpectedError: %v, got: %v", tc.expectedError, err)
473+
}
474+
err = client.Deployments("test-namespace").Delete(context.Background(), baremetalDeploymentName, metav1.DeleteOptions{})
475+
if err != nil {
476+
t.Errorf("Could not delete metal3 test deployment.\n")
477+
}
478+
})
479+
}
480+
481+
}
482+
483+
func TestCheckForBaremetalClusterOperator(t *testing.T) {
484+
testCases := []struct {
485+
testCase string
486+
clusterOperator *osconfigv1.ClusterOperator
487+
expected bool
488+
expectedError bool
489+
}{
490+
{
491+
testCase: cboClusterOperatorName,
492+
clusterOperator: &osconfigv1.ClusterOperator{
493+
TypeMeta: metav1.TypeMeta{
494+
Kind: "ClusterOperator",
495+
APIVersion: "config.openshift.io/v1",
496+
},
497+
ObjectMeta: metav1.ObjectMeta{
498+
Name: cboClusterOperatorName,
499+
},
500+
Status: osconfigv1.ClusterOperatorStatus{
501+
RelatedObjects: []osconfigv1.ObjectReference{
502+
{
503+
Group: "",
504+
Resource: "namespaces",
505+
Name: "openshift-machine-api",
506+
},
507+
},
508+
},
509+
},
510+
expected: true,
511+
},
512+
{
513+
testCase: "invalidCO",
514+
clusterOperator: &osconfigv1.ClusterOperator{
515+
ObjectMeta: metav1.ObjectMeta{
516+
Name: "invalidCO",
517+
},
518+
},
519+
expected: false,
520+
},
521+
}
522+
for _, tc := range testCases {
523+
t.Run(string(tc.testCase), func(t *testing.T) {
524+
var osClient *fakeos.Clientset
525+
osClient = fakeos.NewSimpleClientset(tc.clusterOperator)
526+
_, err := osClient.ConfigV1().ClusterOperators().Create(context.Background(), tc.clusterOperator, metav1.CreateOptions{})
527+
if err != nil && !apierrors.IsAlreadyExists(err) {
528+
t.Fatalf("Unable to create ClusterOperator for test: %v", err)
529+
}
530+
exists, err := checkForBaremetalClusterOperator(osClient)
531+
if exists != tc.expected {
532+
t.Errorf("Expected: %v, got: %v", tc.expected, exists)
533+
}
534+
if tc.expectedError != (err != nil) {
535+
t.Errorf("ExpectedError: %v, got: %v", tc.expectedError, err)
536+
}
537+
err = osClient.ConfigV1().ClusterOperators().Delete(context.Background(), tc.testCase, metav1.DeleteOptions{})
379538
})
380539
}
381540
}

pkg/operator/operator.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,10 @@ const (
3535
// 5ms, 10ms, 20ms, 40ms, 80ms, 160ms, 320ms, 640ms, 1.3s, 2.6s, 5.1s, 10.2s, 20.4s, 41s, 82s
3636
maxRetries = 15
3737
maoOwnedAnnotation = "machine.openshift.io/owned"
38+
// Indicates that the metal3 deployment is being managed by cluster-baremetal-operator
39+
cboOwnedAnnotation = "baremetal.openshift.io/owned"
40+
// The name of the clusteroperator for cluster-baremetal-operator
41+
cboClusterOperatorName = "baremetal"
3842
)
3943

4044
// Operator defines machine api operator.

pkg/operator/sync.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,23 @@ func (optr *Operator) syncMutatingWebhook() error {
198198
}
199199

200200
func (optr *Operator) syncBaremetalControllers(config *OperatorConfig, configName string) error {
201+
// Stand down if cluster-bare-metal operator has claimed the metal3 deployment
202+
// and if the baremetal clusteroperator exists
203+
maoOwned, err := checkMetal3DeploymentMAOOwned(optr.kubeClient.AppsV1(), config)
204+
if err != nil {
205+
return err
206+
}
207+
if !maoOwned {
208+
cboExists, err := checkForBaremetalClusterOperator(optr.osClient)
209+
if err != nil {
210+
return err
211+
}
212+
if cboExists {
213+
glog.Infof("cluster-baremetal-operator is running and managing the Metal3 deployment, standing down.")
214+
return nil
215+
}
216+
}
217+
201218
// Try to get baremetal provisioning config from a CR
202219
baremetalProvisioningConfig, err := getBaremetalProvisioningConfig(optr.dynamicClient, configName)
203220
if err == nil && baremetalProvisioningConfig == nil {

0 commit comments

Comments
 (0)