Skip to content

Commit a04c99e

Browse files
pruivopruivo
authored
Operator: new CR status condition for upgrades
Closes #37220 Signed-off-by: Pedro Ruivo <[email protected]>
1 parent ad28af1 commit a04c99e

File tree

12 files changed

+156
-42
lines changed

12 files changed

+156
-42
lines changed

‎docs/guides/operator/advanced-configuration.adoc‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ spec:
153153

154154
The unsupported podTemplate may be used to override the default probes.
155155

156-
In particular the default startup probe timeout of 10 minutes may be too short in scenarios where there is a long-running migration.
156+
In particular the default startup probe timeout of 10 minutes may be too short in scenarios where there is a long-running migration.
157157

158158
If your instances encounter this startup failure or if you wish to proactively prevent such a startup failure from occurring, then the startup probe timeout should be increased.
159159

@@ -526,4 +526,28 @@ When the image field changes, the StatefulSet is scaled down before applying the
526526

527527
|===
528528

529+
===== CR Statuses =====
530+
531+
The Keycloak CR status of type `RecreateUpdateUsed` indicates the type of update strategy employed during the last update operation.
532+
The `lastTransitionTime` field indicates when the last update occurred.
533+
534+
[%autowidth]
535+
.Condition statuses
536+
|===
537+
|Status |Description
538+
539+
m|Unknown
540+
|The initial state.
541+
It means no update has taken place.
542+
543+
m|False
544+
|The rolling update type was used in the last update.
545+
546+
m|True
547+
|The recreate update type was used in the last update.
548+
The `message` field explains why this strategy was chosen.
549+
550+
|===
551+
552+
529553
</@tmpl.guide>

‎operator/src/main/java/org/keycloak/operator/controllers/KeycloakController.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,8 @@ public UpdateControl<Keycloak> reconcile(Keycloak kc, Context<Keycloak> context)
134134
ContextUtils.storeCurrentStatefulSet(context, existingDeployment);
135135
ContextUtils.storeDesiredStatefulSet(context, new KeycloakDeploymentDependentResource().desired(kc, context));
136136

137-
var upgradeLogicControl = upgradeLogicFactory.create(kc, context)
138-
.decideUpgrade();
137+
var upgradeLogic = upgradeLogicFactory.create(kc, context);
138+
var upgradeLogicControl = upgradeLogic.decideUpgrade();
139139
if (upgradeLogicControl.isPresent()) {
140140
Log.debug("--- Reconciliation interrupted due to upgrade logic");
141141
return upgradeLogicControl.get();
@@ -147,6 +147,7 @@ public UpdateControl<Keycloak> reconcile(Keycloak kc, Context<Keycloak> context)
147147
var statusAggregator = new KeycloakStatusAggregator(kc.getStatus(), kc.getMetadata().getGeneration());
148148

149149
updateStatus(kc, existingDeployment, statusAggregator, context);
150+
upgradeLogic.updateStatus(statusAggregator);
150151
var status = statusAggregator.build();
151152

152153
Log.debug("--- Reconciliation finished successfully");

‎operator/src/main/java/org/keycloak/operator/crds/v2alpha1/deployment/KeycloakStatusAggregator.java‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ public class KeycloakStatusAggregator {
3535
private final KeycloakStatusCondition readyCondition = new KeycloakStatusCondition();
3636
private final KeycloakStatusCondition hasErrorsCondition = new KeycloakStatusCondition();
3737
private final KeycloakStatusCondition rollingUpdate = new KeycloakStatusCondition();
38+
private final KeycloakStatusCondition updateType = new KeycloakStatusCondition();
3839

3940
private final List<String> notReadyMessages = new ArrayList<>();
4041
private final List<String> errorMessages = new ArrayList<>();
@@ -71,6 +72,8 @@ public KeycloakStatusAggregator(KeycloakStatus current, Long generation) {
7172
hasErrorsCondition.setType(KeycloakStatusCondition.HAS_ERRORS);
7273

7374
rollingUpdate.setType(KeycloakStatusCondition.ROLLING_UPDATE);
75+
76+
updateType.setType(KeycloakStatusCondition.UPDATE_TYPE);
7477
}
7578

7679
public KeycloakStatusAggregator addNotReadyMessage(String message) {
@@ -100,6 +103,12 @@ public KeycloakStatusAggregator addRollingUpdateMessage(String message) {
100103
return this;
101104
}
102105

106+
public void addUpgradeType(boolean recreate, String message) {
107+
updateType.setStatus(recreate);
108+
updateType.setObservedGeneration(observedGeneration);
109+
updateType.setMessage(message);
110+
}
111+
103112
/**
104113
* Apply non-condition changes to the status
105114
*/
@@ -142,10 +151,11 @@ public KeycloakStatus build() {
142151
updateConditionFromExisting(readyCondition, existingConditions, now);
143152
updateConditionFromExisting(hasErrorsCondition, existingConditions, now);
144153
updateConditionFromExisting(rollingUpdate, existingConditions, now);
154+
updateConditionFromExisting(updateType, existingConditions, now);
145155

146156
return statusBuilder
147157
.withObservedGeneration(observedGeneration)
148-
.withConditions(List.of(readyCondition, hasErrorsCondition, rollingUpdate))
158+
.withConditions(List.of(readyCondition, hasErrorsCondition, rollingUpdate, updateType))
149159
.build();
150160
}
151161

‎operator/src/main/java/org/keycloak/operator/crds/v2alpha1/deployment/KeycloakStatusCondition.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,4 +26,5 @@ public class KeycloakStatusCondition extends StatusCondition {
2626
public static final String READY = "Ready";
2727
public static final String HAS_ERRORS = "HasErrors";
2828
public static final String ROLLING_UPDATE = "RollingUpdate";
29+
public static final String UPDATE_TYPE = "RecreateUpdateUsed";
2930
}

‎operator/src/main/java/org/keycloak/operator/upgrade/UpgradeLogic.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import io.javaoperatorsdk.operator.api.reconciler.Context;
2424
import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;
2525
import org.keycloak.operator.crds.v2alpha1.deployment.Keycloak;
26+
import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakStatusAggregator;
2627

2728
/**
2829
* An API to implement to handle Keycloak CR updates.
@@ -48,4 +49,11 @@ public interface UpgradeLogic {
4849
*/
4950
Optional<UpdateControl<Keycloak>> decideUpgrade();
5051

52+
/**
53+
* Updates the Keycloak CR status.
54+
*
55+
* @param statusAggregator The {@link KeycloakStatusAggregator} to update.
56+
*/
57+
void updateStatus(KeycloakStatusAggregator statusAggregator);
58+
5159
}

‎operator/src/main/java/org/keycloak/operator/upgrade/impl/AutoUpgradeLogic.java‎

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ Optional<UpdateControl<Keycloak>> onUpgrade() {
6969
if (pod.isEmpty()) {
7070
// TODO some cases the pod is removed. Do we start over or use recreate update?
7171
Log.warn("Pod for Update Job not found.");
72-
decideRecreateUpgrade();
72+
decideRecreateUpgrade("The Pod running update-compatibility command not found.");
7373
return Optional.empty();
7474
}
7575

@@ -99,12 +99,17 @@ private void checkUpgradeType(Pod pod) {
9999
.map(AutoUpgradeLogic::exitCode);
100100
if (initContainerExitCode.isEmpty()) {
101101
Log.warn("InitContainer not found for Update Job.");
102-
decideRecreateUpgrade();
102+
decideRecreateUpgrade("InitContainer running update-compatibility command not found. Did it crash? Check update job for details.");
103103
return;
104104
}
105105
if (initContainerExitCode.get() != 0) {
106+
if (initContainerExitCode.get() == 4) {
107+
Log.warn("Feature 'rolling-update' not enabled.");
108+
decideRecreateUpgrade("Feature 'rolling-update' not enabled.");
109+
return;
110+
}
106111
Log.warn("InitContainer unexpectedly failed for Update Job.");
107-
decideRecreateUpgrade();
112+
decideRecreateUpgrade("Unexpected update-compatibility command exit code (%s). Check update job for details.".formatted(initContainerExitCode.get()));
108113
return;
109114
}
110115

@@ -113,37 +118,37 @@ private void checkUpgradeType(Pod pod) {
113118
.map(AutoUpgradeLogic::exitCode);
114119
if (containerExitCode.isEmpty()) {
115120
Log.warn("Container not found for Update Job.");
116-
decideRecreateUpgrade();
121+
decideRecreateUpgrade("Container running update-compatibility command not found. Did it crash?");
117122
return;
118123
}
119124
switch (containerExitCode.get()) {
120125
case 0: {
121-
decideRollingUpgrade();
126+
decideRollingUpgrade("Compatible changes detected.");
122127
return;
123128
}
124129
case 1: {
125130
Log.warn("Container has an unexpected error for Update Job");
126-
decideRecreateUpgrade();
131+
decideRecreateUpgrade("Unexpected update-compatibility command error. Check update job for details.");
127132
return;
128133
}
129134
case 2: {
130135
Log.warn("Container has an invalid arguments for Update Job.");
131-
decideRecreateUpgrade();
136+
decideRecreateUpgrade("Invalid arguments in update-compatibility command. Check update job for details.");
132137
return;
133138
}
134139
case 3: {
135140
Log.warn("Rolling Update not possible.");
136-
decideRecreateUpgrade();
141+
decideRecreateUpgrade("Incompatible changes detected. Check update job for details.");
137142
return;
138143
}
139144
case 4: {
140145
Log.warn("Feature 'rolling-update' not enabled.");
141-
decideRecreateUpgrade();
146+
decideRecreateUpgrade("Feature 'rolling-update' not enabled.");
142147
return;
143148
}
144149
default: {
145-
Log.warnf("Unexpected Update Job exit code: " + containerExitCode.get());
146-
decideRecreateUpgrade();
150+
Log.warnf("Unexpected Update Job exit code: %s", containerExitCode.get());
151+
decideRecreateUpgrade("Unexpected update-compatibility command exit code (%s). Check update job for details.".formatted(containerExitCode.get()));
147152
}
148153
}
149154
}

‎operator/src/main/java/org/keycloak/operator/upgrade/impl/BaseUpgradeLogic.java‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import java.util.Map;
2121
import java.util.Objects;
2222
import java.util.Optional;
23+
import java.util.function.Consumer;
2324
import java.util.function.Function;
2425
import java.util.stream.Collectors;
2526

@@ -33,6 +34,7 @@
3334
import org.keycloak.operator.controllers.KeycloakDeploymentDependentResource;
3435
import org.keycloak.operator.crds.v2alpha1.CRDUtils;
3536
import org.keycloak.operator.crds.v2alpha1.deployment.Keycloak;
37+
import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakStatusAggregator;
3638
import org.keycloak.operator.upgrade.UpgradeLogic;
3739
import org.keycloak.operator.upgrade.UpgradeType;
3840

@@ -46,6 +48,7 @@ abstract class BaseUpgradeLogic implements UpgradeLogic {
4648

4749
protected final Context<Keycloak> context;
4850
protected final Keycloak keycloak;
51+
private Consumer<KeycloakStatusAggregator> statusConsumer = unused -> {};
4952

5053
BaseUpgradeLogic(Context<Keycloak> context, Keycloak keycloak) {
5154
this.context = context;
@@ -73,28 +76,35 @@ public final Optional<UpdateControl<Keycloak>> decideUpgrade() {
7376
return onUpgrade();
7477
}
7578

79+
@Override
80+
public final void updateStatus(KeycloakStatusAggregator statusAggregator) {
81+
statusConsumer.accept(statusAggregator);
82+
}
83+
7684
/**
7785
* Concrete upgrade logic should be implemented here.
7886
* <p>
7987
* Use {@link ContextUtils#getCurrentStatefulSet(Context)} and/or
8088
* {@link ContextUtils#getDesiredStatefulSet(Context)} to get the current and the desired {@link StatefulSet},
8189
* respectively.
8290
* <p>
83-
* Use the methods {@link #decideRecreateUpgrade()} or {@link #decideRollingUpgrade()} to use one of the available
91+
* Use the methods {@link #decideRecreateUpgrade(String)} or {@link #decideRollingUpgrade(String)} to use one of the available
8492
* upgrade logics.
8593
*
8694
* @return An {@link UpdateControl} if the reconciliation must be interrupted before updating the
8795
* {@link StatefulSet}.
8896
*/
8997
abstract Optional<UpdateControl<Keycloak>> onUpgrade();
9098

91-
void decideRollingUpgrade() {
92-
Log.debug("Decided rolling upgrade type.");
99+
void decideRollingUpgrade(String reason) {
100+
Log.debugf("Decided rolling upgrade type. Reason: %s", reason);
101+
statusConsumer = status -> status.addUpgradeType(false, reason);
93102
ContextUtils.storeUpgradeType(context, UpgradeType.ROLLING);
94103
}
95104

96-
void decideRecreateUpgrade() {
97-
Log.debug("Decided recreate upgrade type.");
105+
void decideRecreateUpgrade(String reason) {
106+
Log.debugf("Decided recreate upgrade type. Reason: %s", reason);
107+
statusConsumer = status -> status.addUpgradeType(true, reason);
98108
ContextUtils.storeUpgradeType(context, UpgradeType.RECREATE);
99109
}
100110

@@ -124,7 +134,6 @@ private static boolean isEnvEquals(Container actual, Container desired) {
124134

125135
private static Map<String, EnvVar> envVars(Container container) {
126136
// The operator only sets value or secrets. Any other combination is from unsupported pod template.
127-
//noinspection DataFlowIssue
128137
return container.getEnv().stream()
129138
.filter(envVar -> !envVar.getName().equals(KeycloakDeploymentDependentResource.POD_IP))
130139
.collect(Collectors.toMap(EnvVar::getName, Function.identity()));

‎operator/src/main/java/org/keycloak/operator/upgrade/impl/ForceRecreateUpgradeLogic.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ public ForceRecreateUpgradeLogic(Context<Keycloak> context, Keycloak keycloak) {
3737

3838
@Override
3939
Optional<UpdateControl<Keycloak>> onUpgrade() {
40-
decideRecreateUpgrade();
40+
decideRecreateUpgrade("Strategy ForceRecreate configured.");
4141
return Optional.empty();
4242
}
4343
}

‎operator/src/main/java/org/keycloak/operator/upgrade/impl/RecreateOnImageChangeUpgradeLogic.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,9 @@ Optional<UpdateControl<Keycloak>> onUpgrade() {
4747
var desiredImage = extractImage(ContextUtils.getDesiredStatefulSet(context));
4848

4949
if (Objects.equals(currentImage, desiredImage)) {
50-
decideRollingUpgrade();
50+
decideRollingUpgrade("Image unchanged.");
5151
} else {
52-
decideRecreateUpgrade();
52+
decideRecreateUpgrade("Image changed %s -> %s".formatted(currentImage, desiredImage));
5353
}
5454
return Optional.empty();
5555
}

‎operator/src/test/java/org/keycloak/operator/testsuite/integration/BaseOperatorTest.java‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,7 @@
4040
import io.fabric8.kubernetes.client.utils.Serialization;
4141
import io.javaoperatorsdk.operator.Operator;
4242
import io.javaoperatorsdk.operator.api.config.BaseConfigurationService;
43-
import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
44-
import io.javaoperatorsdk.operator.api.config.Utils;
45-
import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceConfigurationResolver;
46-
import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec;
4743
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
48-
import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent;
49-
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
50-
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResourceFactory;
5144
import io.quarkiverse.operatorsdk.runtime.QuarkusConfigurationService;
5245
import io.quarkus.logging.Log;
5346
import io.quarkus.test.junit.callback.QuarkusTestAfterEachCallback;
@@ -175,7 +168,7 @@ private static void createRBACresourcesAndOperatorDeployment() throws FileNotFou
175168
K8sUtils.set(k8sclient, new FileInputStream(TARGET_KUBERNETES_GENERATED_YML_FOLDER + deploymentTarget + ".yml"), obj -> {
176169
if (obj instanceof ClusterRoleBinding) {
177170
((ClusterRoleBinding)obj).getSubjects().forEach(s -> s.setNamespace(namespace));
178-
} else if (obj instanceof RoleBinding && "keycloak-operator-view".equals(((RoleBinding)obj).getMetadata().getName())) {
171+
} else if (obj instanceof RoleBinding && "keycloak-operator-view".equals(obj.getMetadata().getName())) {
179172
return null; // exclude this role since it's not present in olm
180173
} else if (obj instanceof Deployment) {
181174
// set values useful for testing - TODO: could drive this in some way from the test/resource/application.properties
@@ -310,8 +303,8 @@ public void afterEach(QuarkusTestMethodContext context) {
310303
Method testMethod = context.getTestMethod();
311304
if (context.getTestStatus().getTestErrorCause() == null
312305
|| context.getTestStatus().getTestErrorCause() instanceof TestAbortedException
313-
|| !Stream.of(context.getTestStatus().getTestErrorCause().getStackTrace())
314-
.anyMatch(ste -> ste.getMethodName().equals(testMethod.getName())
306+
|| Stream.of(context.getTestStatus().getTestErrorCause().getStackTrace())
307+
.noneMatch(ste -> ste.getMethodName().equals(testMethod.getName())
315308
&& ste.getClassName().equals(testMethod.getDeclaringClass().getName()))) {
316309
return;
317310
}
@@ -325,7 +318,7 @@ public void afterEach(QuarkusTestMethodContext context) {
325318
log(k8sclient.apps().deployments().withName("keycloak-operator"), Deployment::getStatus, false);
326319
}
327320
logFailed(k8sclient.apps().statefulSets().withName(POSTGRESQL_NAME), StatefulSet::getStatus);
328-
k8sclient.pods().withLabel("app", "keycloak-realm-import").list().getItems().stream()
321+
k8sclient.pods().withLabel("app", "keycloak-realm-import").list().getItems()
329322
.forEach(pod -> log(k8sclient.pods().resource(pod), Pod::getStatus, false));
330323
} finally {
331324
cleanup();
@@ -380,6 +373,13 @@ private void logFailedKeycloaks() {
380373
threadDump(p);
381374
});
382375
}
376+
var job = k8sclient.batch().v1().jobs()
377+
.inNamespace(kc.getMetadata().getNamespace())
378+
.withName(KeycloakUpdateJobDependentResource.jobName(kc))
379+
.get();
380+
if (job != null) {
381+
Log.warnf("Keycloak Update Job \"%s\" %s", job.getMetadata().getName(), Serialization.asYaml(job.getStatus()));
382+
}
383383
});
384384
}
385385

@@ -421,7 +421,7 @@ private void logEvents() {
421421
var entry = grouped.next();
422422
Log.logf("Normal".equals(entry.getValue().get(0).getType()) ? Level.INFO : Level.WARN,
423423
"Event last seen %s repeated %s times - %s", getTime(entry.getValue().get(0)),
424-
entry.getValue().size(), entry.getKey().stream().collect(Collectors.joining(" ")));
424+
entry.getValue().size(), String.join(" ", entry.getKey()));
425425
}
426426
}
427427

0 commit comments

Comments
 (0)