Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ACM-5994 Reinstall operator when deployment arguments are mismatched #249

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions pkg/install/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@ import (
"context"
"fmt"
"reflect"
"strings"
"time"

"github.com/go-logr/logr"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/util/wait"
"sigs.k8s.io/controller-runtime/pkg/client"

appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/types"

Expand Down Expand Up @@ -124,6 +126,11 @@ func (c *UpgradeController) installOptionsChanged() bool {
return true
}

// check for changes in hypershift operator deployment arguments
operatorDeployment, err := c.getDeployment()
if err == nil && c.operatorArgMismatch(operatorDeployment) {
return true
}
return false
}

Expand Down Expand Up @@ -175,3 +182,42 @@ func (c *UpgradeController) configmapDataChanged(oldCM, newCM corev1.ConfigMap,
}
return false
}

func (c *UpgradeController) getDeployment() (appsv1.Deployment, error) {
deployment := &appsv1.Deployment{}
nsn := types.NamespacedName{Namespace: util.HypershiftOperatorNamespace, Name: util.HypershiftOperatorName}
err := c.spokeUncachedClient.Get(c.ctx, nsn, deployment)
if err != nil {
c.log.Error(err, "failed to get operater deployment: ")
return *deployment, err
}

return *deployment, nil
}

func (c *UpgradeController) operatorArgMismatch(dep appsv1.Deployment) bool {
//If secret exists but operator does not have secret args, add secret args to deployment

//Attempt to retrieve oidc bucket secret
oidcExists := false
bucketSecretKey := types.NamespacedName{Name: util.HypershiftBucketSecretName, Namespace: c.clusterName}
se := &corev1.Secret{}
if err := c.hubClient.Get(c.ctx, bucketSecretKey, se); err == nil {
oidcExists = true
} else {
return false
}

args := dep.Spec.Template.Spec.Containers[0].Args
oidcArgExists := false
for arg := range args {
if strings.Contains(args[arg], "--oidc-storage-provider-s3-bucket-name") {
oidcArgExists = true
break
}
}
if oidcExists != oidcArgExists {
c.log.Info("hypershift operator has mismatch with oidc secrets, reinstalling operator")
}
return oidcExists != oidcArgExists
}
86 changes: 86 additions & 0 deletions pkg/install/upgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package install

import (
"context"
"fmt"
"testing"
"time"

Expand Down Expand Up @@ -614,3 +615,88 @@ func TestInstallFlagChanges(t *testing.T) {

controller.Stop()
}

func TestDeploymentArgMismatch(t *testing.T) {
ctx := context.Background()

zapLog, _ := zap.NewDevelopment()
client := initClient()
controller := &UpgradeController{
spokeUncachedClient: client,
hubClient: client,
log: zapr.NewLogger(zapLog),
addonNamespace: "addon",
operatorImage: "my-test-image",
clusterName: "cluster1",
pullSecret: "pull-secret",
hypershiftInstallExecutor: &HypershiftTestCliExecutor{},
bucketSecret: corev1.Secret{},
}

localOidcSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: util.HypershiftBucketSecretName,
Namespace: controller.clusterName,
},
Data: map[string][]byte{
"bucket": []byte(`my-bucket`),
"region": []byte(`us-east-1`),
"credentials": []byte(`myCredential`),
},
}
operatorDeployment := &appsv1.Deployment{
TypeMeta: metav1.TypeMeta{
Kind: "Deployment",
APIVersion: "apps/v1",
},
ObjectMeta: metav1.ObjectMeta{
Name: "operator",
Namespace: "hypershift",
},
Spec: appsv1.DeploymentSpec{
Template: corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
Containers: []corev1.Container{{
Name: "nginx",
Image: "nginx:1.14.2",
Ports: []corev1.ContainerPort{{ContainerPort: 80}},
}},
},
},
},
}

//Create deployment
controller.hubClient.Create(ctx, operatorDeployment)
defer controller.hubClient.Delete(ctx, operatorDeployment)

defer deleteAllInstallJobs(ctx, controller.spokeUncachedClient, controller.addonNamespace)

// create oidc secret
controller.hubClient.Create(ctx, localOidcSecret)
assert.Eventually(t, func() bool {
theSecret := &corev1.Secret{}
err := controller.hubClient.Get(ctx, types.NamespacedName{Namespace: controller.clusterName, Name: util.HypershiftBucketSecretName}, theSecret)
return err == nil
}, 10*time.Second, 1*time.Second, "The test oidc secret was created successfully")

// Installing first time, reinstall needed
controller.Start()
assert.Eventually(t, func() bool {
return controller.reinstallNeeded
}, 10*time.Second, 1*time.Second, "First time install, \"reinstall\" needed")
controller.Stop()

//Remove args from deployment
operatorDeployment.Spec.Template.Spec.Containers[0].Args = []string{"sample-arg-no-oidc"}
if err := controller.hubClient.Update(ctx, operatorDeployment); err != nil {
fmt.Println("could not update deployment")
}
// oidc secret exists but not present in deployment args, should reinstall
controller.Start()
assert.Eventually(t, func() bool {
return controller.reinstallNeeded
}, 10*time.Second, 1*time.Second, "The oidc secret exists but not among operator args. The hypershift operator needs to be re-installed")
controller.Stop()

}
2 changes: 1 addition & 1 deletion test/e2e/install_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ var _ = ginkgo.Describe("Install", func() {
for _, p := range podList.Items {
if strings.HasPrefix(p.Name, "hypershift-addon-agent") {
addonAgentPod = &p
ginkgo.By("Found addon agent pod" + p.Name)
ginkgo.By("Found addon agent pod " + p.Name)

break
}
Expand Down