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

az aro update CredentialsRequest hotfix #3325

Merged
4 changes: 4 additions & 0 deletions pkg/cluster/arooperator.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ func (m *manager) ensureAROOperatorRunningDesiredVersion(ctx context.Context) (b
return true, nil
}

func (m *manager) ensureCredentialsRequest(ctx context.Context) error {
return m.aroOperatorDeployer.CreateOrUpdateCredentialsRequest(ctx)
}

func (m *manager) renewMDSDCertificate(ctx context.Context) error {
return m.aroOperatorDeployer.RenewMDSDCertificate(ctx)
}
Expand Down
8 changes: 7 additions & 1 deletion pkg/cluster/condition.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
configv1 "github.com/openshift/api/config/v1"
consoleapi "github.com/openshift/console-operator/pkg/api"
corev1 "k8s.io/api/core/v1"
kerrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

Expand Down Expand Up @@ -101,7 +102,7 @@ func isOperatorAvailable(operator *configv1.ClusterOperator) bool {
// is true if the CredentialsRequest has been reconciled within the past 5 minutes.
// Checking for a change to the lastSyncCloudCredsSecretResourceVersion attribute of the CredentialRequest's status would be a neater way of checking
// whether it was reconciled, but we would would have to save the value prior to updating the kube-system/azure-credentials Secret so that we'd have
// and old value to compare to.
// an old value to compare to.
func (m *manager) aroCredentialsRequestReconciled(ctx context.Context) (bool, error) {
// If the CSP hasn't been updated, the CredentialsRequest does not need to be reconciled.
secret, err := m.servicePrincipalUpdated(ctx)
Expand All @@ -113,6 +114,11 @@ func (m *manager) aroCredentialsRequestReconciled(ctx context.Context) (bool, er

u, err := m.dynamiccli.Resource(CredentialsRequestGroupVersionResource).Namespace("openshift-cloud-credential-operator").Get(ctx, "openshift-azure-operator", metav1.GetOptions{})
if err != nil {
// If the CredentialsRequest is not found, it may have just recently been reconciled.
// Return nil to retry until we hit the condition timeout.
kimorris27 marked this conversation as resolved.
Show resolved Hide resolved
if kerrors.IsNotFound(err) {
return false, nil
}
return false, err
}

Expand Down
26 changes: 24 additions & 2 deletions pkg/cluster/condition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package cluster

import (
"context"
"errors"
"testing"
"time"

Expand All @@ -18,9 +19,11 @@ import (
"github.com/sirupsen/logrus"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
dynamicfake "k8s.io/client-go/dynamic/fake"
"k8s.io/client-go/kubernetes/fake"
"k8s.io/client-go/kubernetes/scheme"
ktesting "k8s.io/client-go/testing"

"github.com/Azure/ARO-RP/pkg/api"
"github.com/Azure/ARO-RP/pkg/env"
Expand Down Expand Up @@ -341,7 +344,7 @@ func TestAroCredentialsRequestReconciled(t *testing.T) {
want: true,
},
{
name: "Encounter error getting CredentialsRequest",
name: "CredentialsRequest not found",
kubernetescli: func() *fake.Clientset {
secret := getFakeAROSecret("aadClientId", "aadClientSecret")
return fake.NewSimpleClientset(&secret)
Expand All @@ -353,8 +356,27 @@ func TestAroCredentialsRequestReconciled(t *testing.T) {
ClientID: "aadClientId",
ClientSecret: "aadClientSecretNew",
},
want: false,
},
{
name: "Encounter some other error getting the CredentialsRequest",
kubernetescli: func() *fake.Clientset {
secret := getFakeAROSecret("aadClientId", "aadClientSecret")
return fake.NewSimpleClientset(&secret)
},
dynamiccli: func() *dynamicfake.FakeDynamicClient {
dynamiccli := dynamicfake.NewSimpleDynamicClient(scheme.Scheme)
dynamiccli.PrependReactor("get", "*", func(action ktesting.Action) (handled bool, ret runtime.Object, err error) {
return true, &cloudcredentialv1.CredentialsRequest{}, errors.New("Couldn't get CredentialsRequest for arbitrary reason")
})
return dynamiccli
},
spp: api.ServicePrincipalProfile{
ClientID: "aadClientId",
ClientSecret: "aadClientSecretNew",
},
want: false,
wantErrMsg: `credentialsrequests.cloudcredential.openshift.io "openshift-azure-operator" not found`,
wantErrMsg: "Couldn't get CredentialsRequest for arbitrary reason",
},
{
name: "CredentialsRequest is missing status.lastSyncTimestamp",
Expand Down
1 change: 1 addition & 0 deletions pkg/cluster/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ func (m *manager) Update(ctx context.Context) error {
steps.Action(m.configureAPIServerCertificate),
steps.Action(m.configureIngressCertificate),
steps.Action(m.renewMDSDCertificate),
steps.Action(m.ensureCredentialsRequest),
steps.Action(m.updateOpenShiftSecret),
steps.Condition(m.aroCredentialsRequestReconciled, 3*time.Minute, true),
steps.Action(m.updateAROSecret),
Expand Down
23 changes: 23 additions & 0 deletions pkg/operator/deploy/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ var embeddedFiles embed.FS

type Operator interface {
CreateOrUpdate(context.Context) error
CreateOrUpdateCredentialsRequest(context.Context) error
IsReady(context.Context) (bool, error)
Restart(context.Context, []string) error
IsRunningDesiredVersion(context.Context) (bool, error)
Expand Down Expand Up @@ -368,6 +369,28 @@ func (o *operator) CreateOrUpdate(ctx context.Context) error {
return nil
}

// CreateOrUpdateCredentialsRequest just creates/updates the ARO operator's CredentialsRequest
// rather than doing all of the operator's associated Kubernetes resources.
func (o *operator) CreateOrUpdateCredentialsRequest(ctx context.Context) error {
templ, err := template.ParseFS(embeddedFiles, "staticresources/credentialsrequest.yaml")
bennerv marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return err
}

buff := &bytes.Buffer{}
err = templ.Execute(buff, nil)
if err != nil {
return err
}

crUnstructured, err := dynamichelper.DecodeUnstructured(buff.Bytes())
if err != nil {
return err
}

return o.dh.Ensure(ctx, crUnstructured)
}

func (o *operator) RenewMDSDCertificate(ctx context.Context) error {
key, cert := o.env.ClusterGenevaLoggingSecret()
gcsKeyBytes, err := utilpem.Encode(key)
Expand Down
14 changes: 14 additions & 0 deletions pkg/util/mocks/operator/deploy/deploy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions test/e2e/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ type clientSet struct {
HiveRestConfig *rest.Config
Monitoring monitoringclient.Interface
Kubernetes kubernetes.Interface
Client client.Client
MachineAPI machineclient.Interface
MachineConfig mcoclient.Interface
AROClusters aroclient.Interface
Expand Down Expand Up @@ -277,6 +278,11 @@ func newClientSet(ctx context.Context) (*clientSet, error) {
return nil, err
}

controllerRuntimeClient, err := client.New(restconfig, client.Options{})
if err != nil {
return nil, err
}

monitoring, err := monitoringclient.NewForConfig(restconfig)
if err != nil {
return nil, err
Expand Down Expand Up @@ -358,6 +364,7 @@ func newClientSet(ctx context.Context) (*clientSet, error) {
RestConfig: restconfig,
HiveRestConfig: hiveRestConfig,
Kubernetes: cli,
Client: controllerRuntimeClient,
Monitoring: monitoring,
MachineAPI: machineapicli,
MachineConfig: mcocli,
Expand Down
28 changes: 28 additions & 0 deletions test/e2e/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,42 @@ import (

mgmtnetwork "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-08-01/network"
"github.com/Azure/go-autorest/autorest/to"
cloudcredentialv1 "github.com/openshift/cloud-credential-operator/pkg/apis/cloudcredential/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"

mgmtredhatopenshift20220904 "github.com/Azure/ARO-RP/pkg/client/services/redhatopenshift/mgmt/2022-09-04/redhatopenshift"
mgmtredhatopenshift20230701preview "github.com/Azure/ARO-RP/pkg/client/services/redhatopenshift/mgmt/2023-07-01-preview/redhatopenshift"
"github.com/Azure/ARO-RP/pkg/util/stringutils"
)

var _ = Describe("Update clusters", func() {
It("must replace the ARO operator's CredentialsRequest if it has been deleted", func(ctx context.Context) {
crNamespacedName := types.NamespacedName{
Namespace: "openshift-cloud-credential-operator",
Name: "openshift-azure-operator",
}

By("deleting the CredentialsRequest")
cr := &cloudcredentialv1.CredentialsRequest{
ObjectMeta: metav1.ObjectMeta{
Namespace: "openshift-cloud-credential-operator",
Name: "openshift-azure-operator",
},
}
err := clients.Client.Delete(ctx, cr)
Expect(err).NotTo(HaveOccurred())

By("sending the PATCH request to update the cluster")
err = clients.OpenshiftClusters.UpdateAndWait(ctx, vnetResourceGroup, clusterName, mgmtredhatopenshift20220904.OpenShiftClusterUpdate{})
Expect(err).NotTo(HaveOccurred())

By("checking that the CredentialsRequest has been recreated")
cr = &cloudcredentialv1.CredentialsRequest{}
err = clients.Client.Get(ctx, crNamespacedName, cr)
Expect(err).NotTo(HaveOccurred())
})

It("must restart the aro-operator-master Deployment", func(ctx context.Context) {
By("saving the current revision of the aro-operator-master Deployment")
d, err := clients.Kubernetes.AppsV1().Deployments("openshift-azure-operator").Get(ctx, "aro-operator-master", metav1.GetOptions{})
Expand Down
Loading