Skip to content

Commit 841d30b

Browse files
jsenkojsenko
committed
refactor(operator): improve status conditions handling
1 parent 4772a0d commit 841d30b

File tree

4 files changed

+91
-76
lines changed

4 files changed

+91
-76
lines changed

‎operator/controller/src/main/java/io/apicurio/registry/operator/ApicurioRegistry3Reconciler.java

+2-3
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,7 @@ public UpdateControl<ApicurioRegistry3> reconcile(ApicurioRegistry3 primary,
103103

104104
return context.getSecondaryResource(Deployment.class, AppDeploymentDiscriminator.INSTANCE)
105105
.map(deployment -> {
106-
log.info("Updating Apicurio Registry status:");
107-
primary.setStatus(statusUpdater.next(deployment));
106+
statusUpdater.update(deployment);
108107
return UpdateControl.patchStatus(primary);
109108
}).orElseGet(UpdateControl::noUpdate);
110109
}
@@ -114,7 +113,7 @@ public ErrorStatusUpdateControl<ApicurioRegistry3> updateErrorStatus(ApicurioReg
114113
Context<ApicurioRegistry3> context, Exception ex) {
115114
log.error("Status error", ex);
116115
var statusUpdater = new StatusUpdater(apicurioRegistry);
117-
apicurioRegistry.setStatus(statusUpdater.errorStatus(ex));
116+
statusUpdater.updateWithException(ex);
118117
return ErrorStatusUpdateControl.updateStatus(apicurioRegistry);
119118
}
120119

‎operator/controller/src/main/java/io/apicurio/registry/operator/StatusUpdater.java

+59-53
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,14 @@
88
import org.slf4j.Logger;
99
import org.slf4j.LoggerFactory;
1010

11-
import java.time.Instant;
1211
import java.util.Arrays;
1312
import java.util.List;
1413
import java.util.stream.Collectors;
1514

15+
import static io.apicurio.registry.operator.utils.Mapper.toYAML;
16+
import static java.time.Instant.now;
17+
import static java.util.Objects.requireNonNull;
18+
1619
public class StatusUpdater {
1720

1821
private static final Logger log = LoggerFactory.getLogger(StatusUpdater.class);
@@ -22,79 +25,82 @@ public class StatusUpdater {
2225
public static final String STARTED_TYPE = "STARTED";
2326
public static final String UNKNOWN_TYPE = "UNKNOWN";
2427

25-
private ApicurioRegistry3 registry;
28+
private final ApicurioRegistry3 registry;
2629

2730
public StatusUpdater(ApicurioRegistry3 registry) {
2831
this.registry = registry;
2932
}
3033

3134
private Condition defaultCondition() {
3235
var condition = new Condition();
33-
condition.setStatus(ConditionStatus.TRUE);
3436
condition.setObservedGeneration(
3537
registry.getMetadata() == null ? null : registry.getMetadata().getGeneration());
36-
condition.setLastTransitionTime(Instant.now());
38+
condition.setLastTransitionTime(now());
3739
return condition;
3840
}
3941

40-
public ApicurioRegistry3Status errorStatus(Exception e) {
42+
public void updateWithException(Exception e) {
43+
// TODO: Ignore some KubernetesClientException-s that are caused by update conflicts.
4144
var errorCondition = defaultCondition();
4245
errorCondition.setType(ERROR_TYPE);
43-
errorCondition.setMessage(
44-
Arrays.stream(e.getStackTrace()).map(st -> st.toString()).collect(Collectors.joining("\n")));
46+
errorCondition.setStatus(ConditionStatus.TRUE);
4547
errorCondition.setReason("ERROR_STATUS");
48+
errorCondition.setMessage(Arrays.stream(e.getStackTrace()).map(StackTraceElement::toString)
49+
.collect(Collectors.joining("\n"))); // TODO: Make the message more useful.
4650

47-
var status = new ApicurioRegistry3Status();
48-
status.setConditions(List.of(errorCondition));
51+
registry.withStatus().setConditions(List.of(errorCondition));
52+
}
4953

50-
return status;
54+
private Condition getOrCreateCondition(ApicurioRegistry3Status status, String type) {
55+
var conditions = status.getConditions().stream().filter(c -> type.equals(c.getType())).toList();
56+
if (conditions.size() > 1) {
57+
throw new OperatorException("Duplicate conditions: " + conditions);
58+
} else if (conditions.size() == 1) {
59+
return conditions.get(0);
60+
} else {
61+
var newCondition = defaultCondition();
62+
newCondition.setType(type);
63+
status.getConditions().add(newCondition);
64+
return newCondition;
65+
}
5166
}
5267

53-
public ApicurioRegistry3Status next(Deployment deployment) {
54-
log.debug("Setting status based on Deployment: " + deployment);
55-
if (deployment != null && deployment.getStatus() != null) {
56-
if (deployment.getStatus().getConditions().stream()
57-
.anyMatch(condition -> condition.getStatus().equalsIgnoreCase(
58-
ConditionStatus.TRUE.getValue()) && condition.getType().equals("Available"))
59-
&& !registry.withStatus().getConditions().stream()
60-
.anyMatch(condition -> condition.getType().equals(READY_TYPE))) {
61-
var readyCondition = defaultCondition();
62-
readyCondition.setType(READY_TYPE);
63-
readyCondition.setMessage("Deployment is available");
64-
readyCondition.setReason("READY_STATUS");
65-
var conditions = registry.getStatus().getConditions();
66-
conditions.add(readyCondition);
67-
return registry.getStatus();
68-
} else if (deployment.getStatus().getConditions().size() > 0
69-
&& !registry.withStatus().getConditions().stream()
70-
.anyMatch(condition -> condition.getStatus().getValue().equals(STARTED_TYPE))) {
71-
var generation = registry.getMetadata() == null ? null
72-
: registry.getMetadata().getGeneration();
73-
var nextCondition = defaultCondition();
74-
nextCondition.setType(STARTED_TYPE);
75-
nextCondition.setMessage("Deployment conditions:\n" + deployment.getStatus().getConditions()
76-
.stream().map(dc -> dc.getType()).collect(Collectors.joining("\n")));
77-
nextCondition.setReason("DEPLOYMENT_STARTED");
78-
79-
var status = new ApicurioRegistry3Status();
80-
status.setConditions(List.of(nextCondition));
81-
82-
return status;
83-
} else {
84-
var nextCondition = defaultCondition();
85-
nextCondition.setType(UNKNOWN_TYPE);
86-
nextCondition.setMessage("Deployment conditions:\n" + deployment.getStatus().getConditions()
87-
.stream().map(dc -> dc.getType()).collect(Collectors.joining("\n")));
88-
nextCondition.setReason("UNKNOWN_STATUS");
89-
90-
var status = new ApicurioRegistry3Status();
91-
status.setConditions(List.of(nextCondition));
92-
93-
return status;
94-
}
68+
public void update(Deployment deployment) {
69+
requireNonNull(deployment);
70+
log.debug("Setting status based on Deployment:\n{}", toYAML(deployment.getStatus()));
71+
72+
// Remove error condition if present
73+
registry.withStatus().getConditions().removeIf(c -> ERROR_TYPE.equals(c.getType()));
74+
75+
// Ready
76+
var readyCondition = getOrCreateCondition(registry.getStatus(), READY_TYPE);
77+
if (deployment.getStatus() != null && deployment.getStatus().getConditions().stream()
78+
.anyMatch(condition -> condition.getType().equals("Available")
79+
&& condition.getStatus().equalsIgnoreCase(ConditionStatus.TRUE.getValue()))) {
80+
readyCondition.setStatus(ConditionStatus.TRUE);
81+
readyCondition.setReason("DEPLOYMENT_AVAILABLE"); // TODO: Constants
82+
readyCondition.setMessage("App Deployment is available."); // TODO: What about other components?
83+
} else {
84+
readyCondition.setStatus(ConditionStatus.FALSE);
85+
readyCondition.setReason("DEPLOYMENT_NOT_AVAILABLE");
86+
readyCondition.setMessage("App Deployment is not available.");
87+
}
9588

89+
// Started
90+
var startedCondition = getOrCreateCondition(registry.getStatus(), STARTED_TYPE); // TODO: Do we
91+
// actually need
92+
// this?
93+
if (deployment.getStatus() != null && deployment.getStatus().getConditions().size() > 0) {
94+
startedCondition.setStatus(ConditionStatus.TRUE);
95+
startedCondition.setReason("DEPLOYMENT_STARTED");
96+
startedCondition.setMessage("App Deployment conditions: " + deployment.getStatus().getConditions()
97+
.stream().map(dc -> dc.getType() + " = " + dc.getStatus())
98+
.collect(Collectors.joining(", ")) + ".");
9699
} else {
97-
return errorStatus(new OperatorException("Expected deployment not found"));
100+
startedCondition.setStatus(ConditionStatus.UNKNOWN);
101+
startedCondition.setReason("DEPLOYMENT_STATUS_UNKNOWN");
102+
startedCondition.setMessage("No App Deployment conditions available."); // TODO: This does not
103+
// make much sense.
98104
}
99105
}
100106
}

‎operator/controller/src/test/java/io/apicurio/registry/operator/it/StatusUpdaterTest.java

+23-14
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,25 @@
55
import io.apicurio.registry.operator.api.v1.spec.SqlSpec;
66
import io.apicurio.registry.operator.api.v1.spec.StorageSpec;
77
import io.apicurio.registry.operator.api.v1.spec.StorageType;
8+
import io.apicurio.registry.operator.api.v1.status.Condition;
89
import io.apicurio.registry.operator.resource.ResourceFactory;
910
import io.quarkus.test.junit.QuarkusTest;
1011
import org.junit.jupiter.api.Test;
12+
import org.slf4j.Logger;
13+
import org.slf4j.LoggerFactory;
1114

15+
import static io.apicurio.registry.operator.StatusUpdater.READY_TYPE;
16+
import static io.apicurio.registry.operator.StatusUpdater.STARTED_TYPE;
17+
import static io.apicurio.registry.operator.api.v1.status.ConditionStatus.*;
18+
import static io.apicurio.registry.operator.api.v1.status.ConditionStatus.TRUE;
1219
import static org.assertj.core.api.Assertions.assertThat;
1320
import static org.awaitility.Awaitility.await;
1421

1522
@QuarkusTest
1623
public class StatusUpdaterTest extends ITBase {
1724

25+
private static final Logger log = LoggerFactory.getLogger(StatusUpdaterTest.class);
26+
1827
@Test
1928
void testReadyStatusIsReached() {
2029
var registry = ResourceFactory.deserialize("/k8s/examples/simple.apicurioregistry3.yaml",
@@ -23,14 +32,13 @@ void testReadyStatusIsReached() {
2332

2433
client.resource(registry).create();
2534

26-
await().ignoreExceptions().until(() -> {
27-
var status = client.resource(registry).inNamespace(namespace).get().getStatus();
28-
assertThat(status.getConditions().size()).isEqualTo(2);
29-
assertThat(status.getConditions().stream().anyMatch(c -> c.getType().equalsIgnoreCase("ready")))
30-
.isTrue();
31-
assertThat(status.getConditions().stream().anyMatch(c -> c.getType().equalsIgnoreCase("started")))
32-
.isTrue();
33-
return true;
35+
await().ignoreExceptions().untilAsserted(() -> {
36+
var status = client.resource(registry).get().getStatus();
37+
assertThat(status.getConditions()).hasSize(2);
38+
assertThat(status.getConditions().stream().filter(c -> READY_TYPE.equals(c.getType())))
39+
.map(Condition::getStatus).containsExactly(TRUE);
40+
assertThat(status.getConditions().stream().filter(c -> STARTED_TYPE.equals(c.getType())))
41+
.map(Condition::getStatus).containsExactly(TRUE);
3442
});
3543
}
3644

@@ -56,12 +64,13 @@ void testErrorStatusIsTriggered() {
5664

5765
client.resource(registry).create();
5866

59-
await().ignoreExceptions().until(() -> {
60-
var status = client.resource(registry).inNamespace(namespace).get().getStatus();
61-
assertThat(status.getConditions().size()).isEqualTo(1);
62-
assertThat(status.getConditions().stream().anyMatch(c -> c.getType().equalsIgnoreCase("started")))
63-
.isTrue();
64-
return true;
67+
await().ignoreExceptions().untilAsserted(() -> {
68+
var status = client.resource(registry).get().getStatus();
69+
assertThat(status.getConditions()).hasSize(2);
70+
assertThat(status.getConditions().stream().filter(c -> READY_TYPE.equals(c.getType())))
71+
.map(Condition::getStatus).containsExactly(FALSE);
72+
assertThat(status.getConditions().stream().filter(c -> STARTED_TYPE.equals(c.getType())))
73+
.map(Condition::getStatus).containsExactly(TRUE);
6574
});
6675
}
6776
}

‎operator/controller/src/test/java/io/apicurio/registry/operator/unit/StatusUpdaterTest.java

+7-6
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,11 @@
22

33
import io.apicurio.registry.operator.StatusUpdater;
44
import io.apicurio.registry.operator.api.v1.ApicurioRegistry3;
5-
import io.apicurio.registry.operator.api.v1.status.ConditionStatus;
65
import io.fabric8.kubernetes.api.model.ObjectMeta;
76
import org.junit.jupiter.api.Test;
87

8+
import static io.apicurio.registry.operator.StatusUpdater.ERROR_TYPE;
9+
import static io.apicurio.registry.operator.api.v1.status.ConditionStatus.TRUE;
910
import static org.assertj.core.api.Assertions.assertThat;
1011

1112
public class StatusUpdaterTest {
@@ -25,12 +26,12 @@ void shouldReturnAnErrorStatus() {
2526
var su = new StatusUpdater(defaultRegistry);
2627

2728
// Act
28-
var status = su.errorStatus(new RuntimeException("hello world"));
29+
su.updateWithException(new RuntimeException("hello world"));
2930

3031
// Assert
31-
assertThat(status).isNotNull();
32-
assertThat(status.getConditions()).singleElement();
33-
assertThat(status.getConditions().get(0).getStatus()).isEqualTo(ConditionStatus.TRUE);
34-
assertThat(status.getConditions().get(0).getType()).isEqualTo("ERROR");
32+
assertThat(defaultRegistry.getStatus()).isNotNull();
33+
assertThat(defaultRegistry.getStatus().getConditions()).singleElement();
34+
assertThat(defaultRegistry.getStatus().getConditions().get(0).getStatus()).isEqualTo(TRUE);
35+
assertThat(defaultRegistry.getStatus().getConditions().get(0).getType()).isEqualTo(ERROR_TYPE);
3536
}
3637
}

0 commit comments

Comments
 (0)