Skip to content

Commit 52e0c8a

Browse files
thegridmanthegridman
authored
Fixed an issue where the serviceAccount could not be set to "default" (#762)
1 parent 3b386ab commit 52e0c8a

File tree

6 files changed

+118
-16
lines changed

6 files changed

+118
-16
lines changed

‎api/v1/coherenceresourcespec_types.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -823,7 +823,7 @@ func (in *CoherenceResourceSpec) GetImagePullSecrets() []corev1.LocalObjectRefer
823823

824824
// GetServiceAccountName returns the service account name for the cluster.
825825
func (in *CoherenceResourceSpec) GetServiceAccountName() string {
826-
if in != nil && in.ServiceAccountName != DefaultServiceAccount {
826+
if in != nil {
827827
return in.ServiceAccountName
828828
}
829829
return ""

‎api/v1/create_job_test.go‎

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -129,12 +129,12 @@ func TestCreateJobWithEnvVarsFrom(t *testing.T) {
129129

130130
// Create the test deployment
131131
deployment := createTestCoherenceJobDeployment(spec)
132-
// Create expected StatefulSet
132+
// Create expected Job
133133
expected := createMinimalExpectedJob(deployment)
134134

135135
addEnvVarsFromToJob(expected, coh.ContainerNameCoherence, from...)
136136

137-
// assert that the StatefulSet is as expected
137+
// assert that the Job is as expected
138138
assertJobCreation(t, deployment, expected)
139139
}
140140

@@ -162,10 +162,53 @@ func TestAddLifecycleToJobCoherenceContainer(t *testing.T) {
162162

163163
// Create the test deployment
164164
deployment := createTestCoherenceJobDeployment(spec)
165-
// Create expected StatefulSet
165+
// Create expected Job
166166
expected := createMinimalExpectedJob(deployment)
167167
expected.Spec.Template.Spec.Containers[0].Lifecycle = lc
168168

169-
// assert that the StatefulSet is as expected
169+
// assert that the Job is as expected
170170
assertJobCreation(t, deployment, expected)
171171
}
172+
173+
func TestCreateJobWithServiceAccount(t *testing.T) {
174+
spec := coh.CoherenceResourceSpec{
175+
ServiceAccountName: "Foo",
176+
}
177+
178+
// Create the test deployment
179+
deployment := createTestCoherenceJob(spec)
180+
// Create expected Job
181+
jobExpected := createMinimalExpectedJob(deployment)
182+
jobExpected.Spec.Template.Spec.ServiceAccountName = "Foo"
183+
184+
// assert that the Job is as expected
185+
assertJobCreation(t, deployment, jobExpected)
186+
}
187+
188+
func TestCreateJobWithDefaultServiceAccount(t *testing.T) {
189+
spec := coh.CoherenceResourceSpec{
190+
ServiceAccountName: "default",
191+
}
192+
193+
// Create the test deployment
194+
deployment := createTestCoherenceJob(spec)
195+
// Create expected Job
196+
jobExpected := createMinimalExpectedJob(deployment)
197+
jobExpected.Spec.Template.Spec.ServiceAccountName = "default"
198+
199+
// assert that the Job is as expected
200+
assertJobCreation(t, deployment, jobExpected)
201+
}
202+
203+
func TestCreateJobWithoutServiceAccount(t *testing.T) {
204+
spec := coh.CoherenceResourceSpec{}
205+
206+
// Create the test deployment
207+
deployment := createTestCoherenceJob(spec)
208+
// Create expected Job
209+
jobExpected := createMinimalExpectedJob(deployment)
210+
jobExpected.Spec.Template.Spec.ServiceAccountName = ""
211+
212+
// assert that the Job is as expected
213+
assertJobCreation(t, deployment, jobExpected)
214+
}

‎api/v1/create_statefulset_test.go‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -827,3 +827,46 @@ func TestCreateStatefulSetWithMinReadySecondsSetToZero(t *testing.T) {
827827
// assert that the StatefulSet is as expected
828828
assertStatefulSetCreation(t, deployment, stsExpected)
829829
}
830+
831+
func TestCreateStatefulSetWithServiceAccount(t *testing.T) {
832+
spec := coh.CoherenceResourceSpec{
833+
ServiceAccountName: "Foo",
834+
}
835+
836+
// Create the test deployment
837+
deployment := createTestDeployment(spec)
838+
// Create expected StatefulSet
839+
stsExpected := createMinimalExpectedStatefulSet(deployment)
840+
stsExpected.Spec.Template.Spec.ServiceAccountName = "Foo"
841+
842+
// assert that the StatefulSet is as expected
843+
assertStatefulSetCreation(t, deployment, stsExpected)
844+
}
845+
846+
func TestCreateStatefulSetWithDefaultServiceAccount(t *testing.T) {
847+
spec := coh.CoherenceResourceSpec{
848+
ServiceAccountName: "default",
849+
}
850+
851+
// Create the test deployment
852+
deployment := createTestDeployment(spec)
853+
// Create expected StatefulSet
854+
stsExpected := createMinimalExpectedStatefulSet(deployment)
855+
stsExpected.Spec.Template.Spec.ServiceAccountName = "default"
856+
857+
// assert that the StatefulSet is as expected
858+
assertStatefulSetCreation(t, deployment, stsExpected)
859+
}
860+
861+
func TestCreateStatefulSetWithoutServiceAccount(t *testing.T) {
862+
spec := coh.CoherenceResourceSpec{}
863+
864+
// Create the test deployment
865+
deployment := createTestDeployment(spec)
866+
// Create expected StatefulSet
867+
stsExpected := createMinimalExpectedStatefulSet(deployment)
868+
stsExpected.Spec.Template.Spec.ServiceAccountName = ""
869+
870+
// assert that the StatefulSet is as expected
871+
assertStatefulSetCreation(t, deployment, stsExpected)
872+
}

‎controllers/statefulset/statefulset_controller.go‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ const (
4444
EventReasonScale string = "Scaling"
4545

4646
statusHaRetryEnv = "STATUS_HA_RETRY"
47+
48+
lastAppliedConfigAnnotation string = "kubectl.kubernetes.io/last-applied-configuration"
4749
)
4850

4951
// blank assignment to verify that ReconcileStatefulSet implements reconcile.Reconciler.
@@ -478,6 +480,11 @@ func (in *ReconcileStatefulSet) maybePatchStatefulSet(ctx context.Context, deplo
478480
current.Spec.Template.Spec.Containers[0].Args = []string{}
479481
original.Spec.Template.Spec.Containers[0].Args = []string{}
480482

483+
// do not patch the annotation "kubectl.kubernetes.io/last-applied-configuration"
484+
delete(desired.Annotations, lastAppliedConfigAnnotation)
485+
delete(original.Annotations, lastAppliedConfigAnnotation)
486+
delete(current.Annotations, lastAppliedConfigAnnotation)
487+
481488
desiredPodSpec := desired.Spec.Template
482489
currentPodSpec := current.Spec.Template
483490
originalPodSpec := original.Spec.Template
@@ -514,6 +521,12 @@ func (in *ReconcileStatefulSet) maybePatchStatefulSet(ctx context.Context, deplo
514521

515522
// fix the CreationTimestamp so that it is not in the patch
516523
desired.SetCreationTimestamp(current.GetCreationTimestamp())
524+
525+
sa1 := current.Spec.Template.Spec.ServiceAccountName
526+
sa2 := original.Spec.Template.Spec.ServiceAccountName
527+
sa3 := desired.Spec.Template.Spec.ServiceAccountName
528+
logger.Info("**** About to create patch for StatefulSet", "CurrentSA", sa1, "OriginalSA", sa2, "DesiredSA", sa3)
529+
517530
// create the patch to see whether there is anything to update
518531
patch, data, err := in.CreateThreeWayPatch(current.GetName(), original, desired, current, patching.PatchIgnore)
519532
if err != nil {
@@ -529,15 +542,15 @@ func (in *ReconcileStatefulSet) maybePatchStatefulSet(ctx context.Context, deplo
529542
// Check we have the expected number of ready replicas
530543
if readyReplicas != currentReplicas {
531544
logger.Info("Re-queuing update request. StatefulSet Status not all replicas are ready", "Ready", readyReplicas, "CurrentReplicas", currentReplicas)
532-
return reconcile.Result{Requeue: true, RequeueAfter: time.Minute}, nil
545+
return reconcile.Result{RequeueAfter: time.Minute}, nil
533546
}
534547

535548
// perform the StatusHA check...
536549
checker := probe.CoherenceProbe{Client: in.GetClient(), Config: in.GetManager().GetConfig()}
537550
ha := checker.IsStatusHA(ctx, deployment, current)
538551
if !ha {
539552
logger.Info("Coherence cluster is not StatusHA - re-queuing update request.")
540-
return reconcile.Result{Requeue: true, RequeueAfter: time.Minute}, nil
553+
return reconcile.Result{RequeueAfter: time.Minute}, nil
541554
}
542555
} else {
543556
// the user specifically set a forced update!
@@ -549,7 +562,7 @@ func (in *ReconcileStatefulSet) maybePatchStatefulSet(ctx context.Context, deplo
549562
suspended := in.suspendServices(ctx, deployment, current)
550563
switch suspended {
551564
case probe.ServiceSuspendFailed:
552-
return reconcile.Result{Requeue: true, RequeueAfter: time.Minute}, fmt.Errorf("failed to suspend services prior to updating single member deployment")
565+
return reconcile.Result{RequeueAfter: time.Minute}, fmt.Errorf("failed to suspend services prior to updating single member deployment")
553566
case probe.ServiceSuspendSkipped:
554567
logger.Info("Skipping suspension of Coherence services in single member deployment " + deployment.GetName() +
555568
" prior to update StatefulSet")
@@ -566,7 +579,7 @@ func (in *ReconcileStatefulSet) maybePatchStatefulSet(ctx context.Context, deplo
566579
logger.Info("Error patching StatefulSet " + err.Error())
567580
return in.HandleErrAndRequeue(ctx, err, deployment, fmt.Sprintf(FailedToPatchMessage, deployment.GetName(), err.Error()), logger)
568581
case !patched:
569-
return reconcile.Result{Requeue: true}, nil
582+
return reconcile.Result{RequeueAfter: time.Minute}, nil
570583
}
571584

572585
return reconcile.Result{}, nil

‎controllers/status/status_manager.go‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,11 @@ func (sm *StatusManager) UpdateCoherenceStatusPhase(ctx context.Context, namespa
3232
}
3333

3434
// Update the status phase
35-
deployment.Status.Phase = phase
35+
updated := deployment.DeepCopy()
36+
updated.Status.Phase = phase
3637

3738
// Update the resource
38-
err = sm.Client.Status().Update(ctx, deployment)
39+
err = sm.Client.Status().Patch(ctx, deployment, client.MergeFrom(updated))
3940
if err != nil {
4041
return errors.Wrapf(err, "updating status phase for Coherence resource %s/%s", namespacedName.Namespace, namespacedName.Name)
4142
}
@@ -53,11 +54,12 @@ func (sm *StatusManager) UpdateDeploymentStatusHash(ctx context.Context, namespa
5354
}
5455

5556
// Update the status hash
56-
deployment.Status.Hash = hash
57-
deployment.Status.SetVersion(operator.GetVersion())
57+
updated := deployment.DeepCopy()
58+
updated.Status.Hash = hash
59+
updated.Status.SetVersion(operator.GetVersion())
5860

5961
// Update the resource
60-
err = sm.Client.Status().Update(ctx, deployment)
62+
err = sm.Client.Status().Patch(ctx, deployment, client.MergeFrom(updated))
6163
if err != nil {
6264
return errors.Wrapf(err, "updating status hash for Coherence resource %s/%s", namespacedName.Namespace, namespacedName.Name)
6365
}

‎pkg/patching/patcher.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,8 @@ func (in *patcher) ApplyThreeWayPatchWithCallback(ctx context.Context, name stri
176176
in.logger.WithValues().Info(fmt.Sprintf("Patching %s/%s", kind, name), "Patch", string(data))
177177
err := in.mgr.GetClient().Patch(ctx, current, patch)
178178
if err != nil {
179-
return false, errors.Wrapf(err, "failed to patching %s/%s with %s", kind, name, string(data))
179+
in.logger.WithValues().Info(fmt.Sprintf("Failed to patch %s/%s", kind, name), "Patch", string(data), "Error", err.Error())
180+
return false, errors.Wrapf(err, "failed to patch %s/%s with %s", kind, name, string(data))
180181
}
181182

182183
return true, nil
@@ -200,7 +201,7 @@ func (in *patcher) CreateThreeWayPatch(name string, original, desired, current r
200201

201202
// log the patching
202203
kind := current.GetObjectKind().GroupVersionKind().Kind
203-
in.logger.Info(fmt.Sprintf("Created patching for %s/%s", kind, name), "Patch", string(data))
204+
in.logger.Info(fmt.Sprintf("Created patch for %s/%s", kind, name), "Patch", string(data))
204205

205206
return client.RawPatch(in.patchType, data), data, nil
206207
}

0 commit comments

Comments
 (0)