expands the status handling to account for the prior status (#20856)

Closes #20853
This commit is contained in:
Steven Hawkins 2023-06-08 11:09:39 -04:00 committed by GitHub
parent 61968bf747
commit 91a3ab6b87
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 300 additions and 53 deletions

View file

@ -19,6 +19,10 @@ package org.keycloak.operator;
import io.fabric8.kubernetes.client.KubernetesClient;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
/**
* @author Vaclav Muzikar <vmuzikar@redhat.com>
*/
@ -26,4 +30,13 @@ public final class Utils {
public static boolean isOpenShift(KubernetesClient client) {
return client.supports("operator.openshift.io/v1", "OpenShiftAPIServer");
}
/**
* Returns the current timestamp in ISO 8601 format, for example "2019-07-23T09:08:12.356Z".
* @return the current timestamp in ISO 8601 format, for example "2019-07-23T09:08:12.356Z".
*/
public static String iso8601Now() {
return ZonedDateTime.now(ZoneOffset.UTC).format(DateTimeFormatter.ISO_INSTANT);
}
}

View file

@ -98,7 +98,7 @@ public class KeycloakController implements Reconciler<Keycloak>, EventSourceInit
Log.infof("--- Reconciling Keycloak: %s in namespace: %s", kcName, namespace);
var statusAggregator = new KeycloakStatusAggregator();
var statusAggregator = new KeycloakStatusAggregator(kc.getStatus(), kc.getMetadata().getGeneration());
var kcAdminSecret = new KeycloakAdminSecret(client, kc);
kcAdminSecret.createOrUpdateReconciled();
@ -139,10 +139,8 @@ public class KeycloakController implements Reconciler<Keycloak>, EventSourceInit
updateControl = UpdateControl.updateStatus(kc);
}
if (status
.getConditions()
.stream()
.anyMatch(c -> c.getType().equals(KeycloakStatusCondition.READY) && !c.getStatus())) {
if (status.findCondition(KeycloakStatusCondition.READY)
.filter(c -> !Boolean.TRUE.equals(c.getStatus())).isPresent()) {
updateControl.rescheduleAfter(10, TimeUnit.SECONDS);
}
@ -152,7 +150,7 @@ public class KeycloakController implements Reconciler<Keycloak>, EventSourceInit
@Override
public ErrorStatusUpdateControl<Keycloak> updateErrorStatus(Keycloak kc, Context<Keycloak> context, Exception e) {
Log.error("--- Error reconciling", e);
KeycloakStatus status = new KeycloakStatusAggregator()
KeycloakStatus status = new KeycloakStatusAggregator(kc.getStatus(), kc.getMetadata().getGeneration())
.addErrorMessage("Error performing operations:\n" + e.getMessage())
.build();

View file

@ -18,36 +18,39 @@ package org.keycloak.operator.crds.v2alpha1.deployment;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import io.fabric8.kubernetes.model.annotation.LabelSelector;
import io.fabric8.kubernetes.model.annotation.StatusReplicas;
import io.javaoperatorsdk.operator.api.ObservedGenerationAware;
import io.sundr.builder.annotations.Buildable;
/**
* @author Vaclav Muzikar <vmuzikar@redhat.com>
*/
@Buildable(editableEnabled = false, builderPackage = "io.fabric8.kubernetes.api.builder", lazyCollectionInitEnabled = false)
public class KeycloakStatus {
public class KeycloakStatus implements ObservedGenerationAware {
@LabelSelector
private String selector;
@StatusReplicas
private Integer instances;
private Long observedGeneration;
private List<KeycloakStatusCondition> conditions;
public String getSelector() {
return selector;
}
public void setSelector(String selector) {
this.selector = selector;
}
public Integer getInstances() {
return instances;
}
public void setInstances(Integer instances) {
this.instances = instances;
}
@ -60,18 +63,36 @@ public class KeycloakStatus {
this.conditions = conditions;
}
public Optional<KeycloakStatusCondition> findCondition(String type) {
if (conditions == null || conditions.isEmpty()) {
return Optional.empty();
}
return conditions.stream().filter(c -> type.equals(c.getType())).findFirst();
}
@Override
public Long getObservedGeneration() {
return observedGeneration;
}
@Override
public void setObservedGeneration(Long generation) {
this.observedGeneration = generation;
}
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
KeycloakStatus status = (KeycloakStatus) o;
return Objects.equals(getConditions(), status.getConditions())
return Objects.equals(getConditions(), status.getConditions())
&& Objects.equals(getInstances(), status.getInstances())
&& Objects.equals(getSelector(), status.getSelector());
&& Objects.equals(getSelector(), status.getSelector())
&& Objects.equals(getObservedGeneration(), status.getObservedGeneration());
}
@Override
public int hashCode() {
return Objects.hash(getConditions(), getInstances(), getSelector());
return Objects.hash(getConditions(), getInstances(), getSelector(), getObservedGeneration());
}
}

View file

@ -17,61 +17,90 @@
package org.keycloak.operator.crds.v2alpha1.deployment;
import org.keycloak.operator.Utils;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.stream.Collectors;
/**
* @author Vaclav Muzikar <vmuzikar@redhat.com>
*/
public class KeycloakStatusAggregator {
private final KeycloakStatusCondition readyCondition;
private final KeycloakStatusCondition hasErrorsCondition;
private final KeycloakStatusCondition rollingUpdate;
private final KeycloakStatusCondition readyCondition = new KeycloakStatusCondition();
private final KeycloakStatusCondition hasErrorsCondition = new KeycloakStatusCondition();
private final KeycloakStatusCondition rollingUpdate = new KeycloakStatusCondition();
private final List<String> notReadyMessages = new ArrayList<>();
private final List<String> errorMessages = new ArrayList<>();
private final List<String> rollingUpdateMessages = new ArrayList<>();
private final KeycloakStatusBuilder statusBuilder = new KeycloakStatusBuilder();
public KeycloakStatusAggregator() {
readyCondition = new KeycloakStatusCondition();
private final KeycloakStatusBuilder statusBuilder;
private final Map<String, KeycloakStatusCondition> existingConditions;
private final Long observedGeneration;
/**
* @param generation the observedGeneration for conditions
*/
public KeycloakStatusAggregator(Long generation) {
this(null, generation);
}
/**
* @param current status, which is used as a base for the next conditions
* @param generation the observedGeneration for conditions
*/
public KeycloakStatusAggregator(KeycloakStatus current, Long generation) {
if (current != null) { // 6.7 fabric8 no longer requires this null check
statusBuilder = new KeycloakStatusBuilder(current);
existingConditions = Optional.ofNullable(current.getConditions()).orElse(List.of()).stream().collect(Collectors.toMap(KeycloakStatusCondition::getType, Function.identity()));
} else {
statusBuilder = new KeycloakStatusBuilder();
existingConditions = Map.of();
}
// we're not setting this on the statusBuilder as we're letting the sdk manage that
observedGeneration = generation;
readyCondition.setType(KeycloakStatusCondition.READY);
readyCondition.setStatus(true);
hasErrorsCondition = new KeycloakStatusCondition();
hasErrorsCondition.setType(KeycloakStatusCondition.HAS_ERRORS);
hasErrorsCondition.setStatus(false);
rollingUpdate = new KeycloakStatusCondition();
rollingUpdate.setType(KeycloakStatusCondition.ROLLING_UPDATE);
rollingUpdate.setStatus(false);
}
public KeycloakStatusAggregator addNotReadyMessage(String message) {
readyCondition.setStatus(false);
readyCondition.setObservedGeneration(observedGeneration);
notReadyMessages.add(message);
return this;
}
public KeycloakStatusAggregator addErrorMessage(String message) {
hasErrorsCondition.setStatus(true);
hasErrorsCondition.setObservedGeneration(observedGeneration);
errorMessages.add(message);
return this;
}
public KeycloakStatusAggregator addWarningMessage(String message) {
errorMessages.add("warning: " + message);
hasErrorsCondition.setObservedGeneration(observedGeneration);
return this;
}
public KeycloakStatusAggregator addRollingUpdateMessage(String message) {
rollingUpdate.setStatus(true);
rollingUpdate.setObservedGeneration(observedGeneration);
rollingUpdateMessages.add(message);
return this;
}
/**
* Apply non-condition changes to the status
*/
@ -85,10 +114,58 @@ public class KeycloakStatusAggregator {
}
public KeycloakStatus build() {
readyCondition.setMessage(String.join("\n", notReadyMessages));
hasErrorsCondition.setMessage(String.join("\n", errorMessages));
rollingUpdate.setMessage(String.join("\n", rollingUpdateMessages));
// conditions are only updated in one direction - the following determines if it's appropriate to observe the default / other direction
if (readyCondition.getStatus() == null && !Boolean.TRUE.equals(hasErrorsCondition.getStatus())) {
readyCondition.setStatus(true);
readyCondition.setObservedGeneration(observedGeneration);
}
if (readyCondition.getObservedGeneration() != null) {
readyCondition.setMessage(String.join("\n", notReadyMessages));
}
if (hasErrorsCondition.getStatus() == null && readyCondition.getObservedGeneration() != null) {
hasErrorsCondition.setStatus(false);
hasErrorsCondition.setObservedGeneration(observedGeneration);
}
if (hasErrorsCondition.getObservedGeneration() != null) {
hasErrorsCondition.setMessage(String.join("\n", errorMessages));
}
if (rollingUpdate.getStatus() == null && readyCondition.getObservedGeneration() != null) {
rollingUpdate.setStatus(false);
rollingUpdate.setObservedGeneration(observedGeneration);
}
if (rollingUpdate.getObservedGeneration() != null) {
rollingUpdate.setMessage(String.join("\n", rollingUpdateMessages));
}
String now = Utils.iso8601Now();
updateConditionFromExisting(readyCondition, existingConditions, now);
updateConditionFromExisting(hasErrorsCondition, existingConditions, now);
updateConditionFromExisting(rollingUpdate, existingConditions, now);
return statusBuilder.withConditions(List.of(readyCondition, hasErrorsCondition, rollingUpdate)).build();
}
static void updateConditionFromExisting(KeycloakStatusCondition condition, Map<String, KeycloakStatusCondition> existingConditions, String now) {
var existing = existingConditions.get(condition.getType());
if (existing == null) {
if (condition.getObservedGeneration() != null) {
condition.setLastTransitionTime(now);
}
} else if (condition.getObservedGeneration() == null) {
// carry the existing forward
condition.setLastTransitionTime(existing.getLastTransitionTime());
condition.setObservedGeneration(existing.getObservedGeneration());
condition.setStatus(existing.getStatus());
if (condition.getMessage() == null) {
condition.setMessage(existing.getMessage());
}
} else if (Objects.equals(existing.getStatus(), condition.getStatus())
&& Objects.equals(existing.getMessage(), condition.getMessage())) {
condition.setLastTransitionTime(existing.getLastTransitionTime());
} else {
condition.setLastTransitionTime(now);
}
}
}

View file

@ -31,6 +31,8 @@ public class KeycloakStatusCondition {
private String type;
private Boolean status;
private String message;
private String lastTransitionTime;
private Long observedGeneration;
public String getType() {
return type;
@ -56,17 +58,35 @@ public class KeycloakStatusCondition {
this.message = message;
}
public String getLastTransitionTime() {
return lastTransitionTime;
}
public void setLastTransitionTime(String lastTransitionTime) {
this.lastTransitionTime = lastTransitionTime;
}
public Long getObservedGeneration() {
return observedGeneration;
}
public void setObservedGeneration(Long observedGeneration) {
this.observedGeneration = observedGeneration;
}
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
KeycloakStatusCondition that = (KeycloakStatusCondition) o;
return Objects.equals(getType(), that.getType()) && Objects.equals(getStatus(), that.getStatus()) && Objects.equals(getMessage(), that.getMessage());
return Objects.equals(getType(), that.getType()) && Objects.equals(getStatus(), that.getStatus()) && Objects.equals(getMessage(), that.getMessage())
&& Objects.equals(getLastTransitionTime(), that.getLastTransitionTime())
&& Objects.equals(getObservedGeneration(), that.getObservedGeneration());
}
@Override
public int hashCode() {
return Objects.hash(getType(), getStatus(), getMessage());
return Objects.hash(getType(), getStatus(), getMessage(), getObservedGeneration(), getLastTransitionTime());
}
@Override

View file

@ -79,6 +79,10 @@ public class KeycloakDeploymentTest extends BaseOperatorTest {
Log.info("Checking Keycloak pod has ready replicas == 1");
assertThat(k8sclient.apps().statefulSets().inNamespace(namespace).withName(deploymentName).get().getStatus().getReadyReplicas()).isEqualTo(1);
Log.info("Checking observedGeneration is the same as the spec");
Keycloak latest = k8sclient.resource(kc).get();
assertThat(latest.getMetadata().getGeneration()).isEqualTo(latest.getStatus().getObservedGeneration());
// Delete CR
Log.info("Deleting Keycloak CR and watching cleanup");
k8sclient.resource(kc).delete();

View file

@ -22,6 +22,7 @@ import io.fabric8.kubernetes.api.model.EnvVar;
import io.fabric8.kubernetes.api.model.apps.StatefulSet;
import io.fabric8.kubernetes.api.model.apps.StatefulSetBuilder;
import io.quarkus.test.junit.QuarkusTest;
import org.junit.jupiter.api.Test;
import org.keycloak.common.util.CollectionUtil;
import org.keycloak.operator.Constants;
@ -208,7 +209,7 @@ public class KeycloakDistConfiguratorTest {
private void assertWarningStatusFirstClassFields(KeycloakDistConfigurator distConfig, boolean expectWarning, Collection<String> firstClassFields) {
final String message = "warning: You need to specify these fields as the first-class citizen of the CR: ";
final KeycloakStatusAggregator statusBuilder = new KeycloakStatusAggregator();
final KeycloakStatusAggregator statusBuilder = new KeycloakStatusAggregator(1L);
distConfig.validateOptions(statusBuilder);
final KeycloakStatus status = statusBuilder.build();

View file

@ -17,21 +17,124 @@
package org.keycloak.operator.testsuite.unit;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import org.assertj.core.api.Condition;
import org.junit.jupiter.api.Test;
import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakStatus;
import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakStatusAggregator;
import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakStatusCondition;
import org.keycloak.operator.testsuite.utils.CRAssert;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
public class KeycloakStatusTest {
@Test
public void testEqualityWithScale() {
KeycloakStatus status1 = new KeycloakStatusAggregator().apply(b -> b.withInstances(1)).build();
KeycloakStatus status2 = new KeycloakStatusAggregator().apply(b -> b.withInstances(2)).build();
KeycloakStatus status1 = new KeycloakStatusAggregator(0L).apply(b -> b.withInstances(1)).build();
KeycloakStatus status2 = new KeycloakStatusAggregator(0L).apply(b -> b.withInstances(2)).build();
assertNotEquals(status1, status2);
}
@Test
public void testDefaults() {
KeycloakStatus status = new KeycloakStatusAggregator(1L).build();
CRAssert.assertKeycloakStatusCondition(status, KeycloakStatusCondition.READY, true, "", 1L);
CRAssert.assertKeycloakStatusCondition(status, KeycloakStatusCondition.ROLLING_UPDATE, false, "", 1L);
CRAssert.assertKeycloakStatusCondition(status, KeycloakStatusCondition.HAS_ERRORS, false, "", 1L);
}
@Test
public void testReadyWithWarning() {
KeycloakStatus status = new KeycloakStatusAggregator(1L).addWarningMessage("something's not right").build();
CRAssert.assertKeycloakStatusCondition(status, KeycloakStatusCondition.READY, true, "", 1L);
CRAssert.assertKeycloakStatusCondition(status, KeycloakStatusCondition.HAS_ERRORS, false, "warning: something's not right", 1L); // could also be unknown
}
@Test
public void testNotReady() {
KeycloakStatus status = new KeycloakStatusAggregator(1L).addNotReadyMessage("waiting").build();
CRAssert.assertKeycloakStatusCondition(status, KeycloakStatusCondition.READY, false, "waiting", 1L);
CRAssert.assertKeycloakStatusCondition(status, KeycloakStatusCondition.HAS_ERRORS, false, "", 1L);
}
@Test
public void testReadyRolling() {
KeycloakStatus status = new KeycloakStatusAggregator(1L).addRollingUpdateMessage("rolling").build();
CRAssert.assertKeycloakStatusCondition(status, KeycloakStatusCondition.READY, true, "", 1L);
CRAssert.assertKeycloakStatusCondition(status, KeycloakStatusCondition.ROLLING_UPDATE, true, "rolling", 1L);
}
@Test
public void testError() {
// without prior status, ready and rolling are unknown
KeycloakStatus status = new KeycloakStatusAggregator(1L).addErrorMessage("this is bad").build();
CRAssert.assertKeycloakStatusCondition(status, KeycloakStatusCondition.READY, null, null, null);
CRAssert.assertKeycloakStatusCondition(status, KeycloakStatusCondition.HAS_ERRORS, true, "this is bad", 1L);
CRAssert.assertKeycloakStatusCondition(status, KeycloakStatusCondition.ROLLING_UPDATE, null, null, null);
}
@Test
public void testErrorWithPriorStatus() {
// with prior status, ready and rolling are preserved
KeycloakStatus prior = new KeycloakStatusAggregator(1L).build();
prior.getConditions().stream().forEach(c -> c.setLastTransitionTime("prior"));
KeycloakStatus status = new KeycloakStatusAggregator(prior, 2L).addErrorMessage("this is bad").build();
CRAssert.assertKeycloakStatusCondition(status, KeycloakStatusCondition.READY, true, "", 1L)
.extracting(KeycloakStatusCondition::getLastTransitionTime).isEqualTo("prior");
CRAssert.assertKeycloakStatusCondition(status, KeycloakStatusCondition.HAS_ERRORS, true, "this is bad", 2L)
.extracting(KeycloakStatusCondition::getLastTransitionTime).isNotEqualTo("prior");
CRAssert.assertKeycloakStatusCondition(status, KeycloakStatusCondition.ROLLING_UPDATE, false, "", 1L);
}
@Test
public void testReadyWithPriorStatus() {
// without prior status, ready and rolling are known and keep the transition time
KeycloakStatus prior = new KeycloakStatusAggregator(1L).build();
prior.getConditions().stream().forEach(c -> c.setLastTransitionTime("prior"));
KeycloakStatus status = new KeycloakStatusAggregator(prior, 2L).build();
CRAssert.assertKeycloakStatusCondition(status, KeycloakStatusCondition.READY, true, "", 2L)
.extracting(KeycloakStatusCondition::getLastTransitionTime).isEqualTo("prior");
CRAssert.assertKeycloakStatusCondition(status, KeycloakStatusCondition.HAS_ERRORS, false, "", 2L);
CRAssert.assertKeycloakStatusCondition(status, KeycloakStatusCondition.ROLLING_UPDATE, false, "", 2L);
}
@Test
public void testMessagesChangesLastTransitionTime() {
KeycloakStatus prior = new KeycloakStatusAggregator(1L).build();
prior.getConditions().stream().forEach(c -> {
c.setLastTransitionTime("prior");
c.setMessage("old");
});
KeycloakStatus status = new KeycloakStatusAggregator(prior, 2L).build();
CRAssert.assertKeycloakStatusCondition(status, KeycloakStatusCondition.READY, true, "", 2L).has(new Condition<>(
c -> !c.getLastTransitionTime().equals("prior") && !c.getMessage().equals("old"), "transitioned"));
}
@Test
public void testPreservesScale() {
KeycloakStatus prior = new KeycloakStatusAggregator(1L).apply(b -> b.withObservedGeneration(1L).withInstances(3)).build();
prior.getConditions().stream().forEach(c -> c.setLastTransitionTime("prior"));
KeycloakStatus status = new KeycloakStatusAggregator(prior, 2L).apply(b -> b.withObservedGeneration(2L)).build();
assertEquals(2, status.getObservedGeneration());
assertEquals(3, status.getInstances());
}
}

View file

@ -19,8 +19,11 @@ package org.keycloak.operator.testsuite.utils;
import io.fabric8.kubernetes.client.utils.Serialization;
import io.quarkus.logging.Log;
import org.assertj.core.api.ObjectAssert;
import org.keycloak.operator.crds.v2alpha1.deployment.Keycloak;
import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakStatus;
import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakStatusCondition;
import org.keycloak.operator.crds.v2alpha1.realmimport.KeycloakRealmImport;
import static org.assertj.core.api.Assertions.assertThat;
@ -36,23 +39,30 @@ public final class CRAssert {
public static void assertKeycloakStatusCondition(Keycloak kc, String condition, boolean status, String containedMessage) {
Log.debugf("Asserting CR: %s, condition: %s, status: %s, message: %s", kc.getMetadata().getName(), condition, status, containedMessage);
try {
assertKeycloakStatusCondition(kc.getStatus(), condition, status, containedMessage);
assertKeycloakStatusCondition(kc.getStatus(), condition, status, containedMessage, null);
} catch (Exception e) {
Log.infof("Asserting CR: %s with status:\n%s", kc.getMetadata().getName(), Serialization.asYaml(kc.getStatus()));
throw e;
}
}
public static void assertKeycloakStatusCondition(KeycloakStatus kcStatus, String condition, boolean status) {
assertKeycloakStatusCondition(kcStatus, condition, status, null);
public static void assertKeycloakStatusCondition(KeycloakStatus kcStatus, String condition, Boolean status, String containedMessage) {
assertKeycloakStatusCondition(kcStatus, condition, status, containedMessage, null);
}
public static void assertKeycloakStatusCondition(KeycloakStatus kcStatus, String condition, boolean status, String containedMessage) {
assertThat(kcStatus.getConditions())
.anyMatch(c ->
c.getType().equals(condition) &&
c.getStatus() == status &&
(containedMessage == null || c.getMessage().contains(containedMessage))
);
public static ObjectAssert<KeycloakStatusCondition> assertKeycloakStatusCondition(KeycloakStatus kcStatus, String condition, Boolean status, String containedMessage, Long observedGeneration) {
KeycloakStatusCondition statusCondition = kcStatus.findCondition(condition).orElseThrow();
assertThat(statusCondition.getStatus()).isEqualTo(status);
if (containedMessage != null) {
assertThat(statusCondition.getMessage()).contains(containedMessage);
}
if (observedGeneration != null) {
assertThat(statusCondition.getObservedGeneration()).isEqualTo(observedGeneration);
}
if (status != null) {
assertThat(statusCondition.getLastTransitionTime()).isNotNull();
}
return assertThat(statusCondition);
}
public static void assertKeycloakStatusDoesNotContainMessage(KeycloakStatus kcStatus, String message) {