Skip to content

Commit f38acc8

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #689 from sadasu/check-for-cbo
Bug 1873234: Manage ownership hand-off of metal3 between MAO and CBO for upgrade and downgrade
2 parents 23447c1 + 1a2c56d commit f38acc8

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)