Skip to content

Commit

Permalink
Apply feedback from PR
Browse files Browse the repository at this point in the history
  • Loading branch information
@EricWittmann
EricWittmann committed Jan 29, 2025
1 parent 3c6ba6a commit 8e7d3ca
Show file tree
Hide file tree
Showing 5 changed files with 141 additions and 37 deletions.
20 changes: 14 additions & 6 deletions ...controller/src/main/java/io/apicurio/registry/operator/resource/ActivationConditions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import io.apicurio.registry.operator.api.v1.spec.AppSpec;
import io.apicurio.registry.operator.api.v1.spec.ComponentSpec;
import io.apicurio.registry.operator.api.v1.spec.IngressSpec;
import io.apicurio.registry.operator.api.v1.spec.PodDisruptionSpec;
import io.apicurio.registry.operator.api.v1.spec.StudioUiSpec;
import io.apicurio.registry.operator.api.v1.spec.UiSpec;
import io.apicurio.registry.operator.resource.app.AppIngressResource;
Expand Down Expand Up @@ -52,8 +53,13 @@ public static class AppPodDisruptionBudgetActivationCondition
@Override
public boolean isMet(DependentResource<PodDisruptionBudget, ApicurioRegistry3> resource,
ApicurioRegistry3 primary, Context<ApicurioRegistry3> context) {
Boolean isManaged = ofNullable(primary.getSpec()).map(ApicurioRegistry3Spec::getApp)
.map(ComponentSpec::getManageDisruptionBudget).orElse(Boolean.TRUE);
boolean isEnabled = ofNullable(primary.getSpec()).map(ApicurioRegistry3Spec::getApp)
.map(ComponentSpec::getPodDisruptionBudget).map(PodDisruptionSpec::getEnabled)
.orElse(Boolean.TRUE);
int numReplicas = ofNullable(primary.getSpec()).map(ApicurioRegistry3Spec::getApp)
.map(ComponentSpec::getReplicas).orElse(1);

boolean isManaged = isEnabled && numReplicas > 1;
if (!isManaged) {
((AppPodDisruptionBudgetResource) resource).delete(primary, context);
}
Expand Down Expand Up @@ -84,8 +90,9 @@ public static class UIPodDisruptionBudgetActivationCondition
@Override
public boolean isMet(DependentResource<PodDisruptionBudget, ApicurioRegistry3> resource,
ApicurioRegistry3 primary, Context<ApicurioRegistry3> context) {
Boolean isManaged = ofNullable(primary.getSpec()).map(ApicurioRegistry3Spec::getUi)
.map(ComponentSpec::getManageDisruptionBudget).orElse(Boolean.TRUE);
boolean isManaged = ofNullable(primary.getSpec()).map(ApicurioRegistry3Spec::getUi)
.map(ComponentSpec::getPodDisruptionBudget).map(PodDisruptionSpec::getEnabled)
.orElse(Boolean.TRUE);
if (!isManaged) {
((UIPodDisruptionBudgetResource) resource).delete(primary, context);
}
Expand Down Expand Up @@ -132,8 +139,9 @@ public static class StudioUIPodDisruptionBudgetActivationCondition
@Override
public boolean isMet(DependentResource<PodDisruptionBudget, ApicurioRegistry3> resource,
ApicurioRegistry3 primary, Context<ApicurioRegistry3> context) {
Boolean isManaged = ofNullable(primary.getSpec()).map(ApicurioRegistry3Spec::getStudioUi)
.map(ComponentSpec::getManageDisruptionBudget).orElse(Boolean.TRUE);
boolean isManaged = ofNullable(primary.getSpec()).map(ApicurioRegistry3Spec::getStudioUi)
.map(ComponentSpec::getPodDisruptionBudget).map(PodDisruptionSpec::getEnabled)
.orElse(Boolean.TRUE);
if (!isManaged) {
((StudioUIPodDisruptionBudgetResource) resource).delete(primary, context);
}
Expand Down
65 changes: 42 additions & 23 deletions .../controller/src/test/java/io/apicurio/registry/operator/it/PodDisruptionBudgetITTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@
import io.apicurio.registry.operator.api.v1.ApicurioRegistry3;
import io.apicurio.registry.operator.resource.ResourceFactory;
import io.fabric8.kubernetes.api.model.policy.v1.PodDisruptionBudget;
import io.fabric8.kubernetes.api.model.policy.v1.PodDisruptionBudgetStatus;
import io.quarkus.test.junit.QuarkusTest;
import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.util.Map;
import java.util.stream.Collectors;

@QuarkusTest
Expand All @@ -18,39 +20,56 @@ public class PodDisruptionBudgetITTest extends ITBase {

@Test
void testPodDisruptionBudget() {
ApicurioRegistry3 registry = ResourceFactory
.deserialize("/k8s/examples/simple.apicurioregistry3.yaml", ApicurioRegistry3.class);
ApicurioRegistry3 registry = ResourceFactory.deserialize(
"/k8s/examples/simple-with-studio.apicurioregistry3.yaml", ApicurioRegistry3.class);
registry.getSpec().getApp().setReplicas(2);
client.resource(registry).create();

// Wait for the deployment to exist
checkDeploymentExists(registry, ResourceFactory.COMPONENT_APP, 1);
checkDeploymentExists(registry, ResourceFactory.COMPONENT_APP, 2);

// Check that the two expected PodDisruptionBudget resources were created
PodDisruptionBudget appPDB = checkPodDisruptionBudgetExists(registry, ResourceFactory.COMPONENT_APP);
PodDisruptionBudget uiPDB = checkPodDisruptionBudgetExists(registry, ResourceFactory.COMPONENT_UI);
PodDisruptionBudget studioPDB = checkPodDisruptionBudgetExists(registry,
ResourceFactory.COMPONENT_STUDIO_UI);

// Verify the content of the app component's PDB
Assertions
.assertThat(appPDB.getMetadata().getLabels().entrySet().stream()
.map(l -> l.getKey() + "=" + l.getValue()).collect(Collectors.toSet()))
.contains("app.kubernetes.io/component=app",
"app.kubernetes.io/managed-by=apicurio-registry-operator",
"app.kubernetes.io/name=apicurio-registry");
Assertions
.assertThat(appPDB.getSpec().getSelector().getMatchLabels().entrySet().stream()
.map(l -> l.getKey() + "=" + l.getValue()).collect(Collectors.toSet()))
.contains("app.kubernetes.io/component=app", "app.kubernetes.io/name=apicurio-registry");
assertLabelsContains(appPDB.getMetadata().getLabels(), "app.kubernetes.io/component=app",
"app.kubernetes.io/managed-by=apicurio-registry-operator",
"app.kubernetes.io/name=apicurio-registry");
assertLabelsContains(appPDB.getSpec().getSelector().getMatchLabels(),
"app.kubernetes.io/component=app", "app.kubernetes.io/name=apicurio-registry",
"app.kubernetes.io/instance=" + registry.getMetadata().getName());
PodDisruptionBudgetStatus appPdbStatus = appPDB.getStatus();
Assertions.assertThat(appPdbStatus.getExpectedPods()).isEqualTo(2);
Assertions.assertThat(appPdbStatus.getDisruptionsAllowed()).isEqualTo(1);

// Verify the content of the ui component's PDB
Assertions
.assertThat(uiPDB.getMetadata().getLabels().entrySet().stream()
.map(l -> l.getKey() + "=" + l.getValue()).collect(Collectors.toSet()))
.contains("app.kubernetes.io/component=ui",
"app.kubernetes.io/managed-by=apicurio-registry-operator",
"app.kubernetes.io/name=apicurio-registry");
Assertions
.assertThat(uiPDB.getSpec().getSelector().getMatchLabels().entrySet().stream()
.map(l -> l.getKey() + "=" + l.getValue()).collect(Collectors.toSet()))
.contains("app.kubernetes.io/component=ui", "app.kubernetes.io/name=apicurio-registry");
assertLabelsContains(uiPDB.getMetadata().getLabels(), "app.kubernetes.io/component=ui",
"app.kubernetes.io/managed-by=apicurio-registry-operator",
"app.kubernetes.io/name=apicurio-registry");
assertLabelsContains(uiPDB.getSpec().getSelector().getMatchLabels(), "app.kubernetes.io/component=ui",
"app.kubernetes.io/name=apicurio-registry",
"app.kubernetes.io/instance=" + registry.getMetadata().getName());
PodDisruptionBudgetStatus uiPdbStatus = uiPDB.getStatus();
Assertions.assertThat(uiPdbStatus.getExpectedPods()).isEqualTo(1);
Assertions.assertThat(uiPdbStatus.getDisruptionsAllowed()).isEqualTo(0);

// Verify the content of the studio component's PDB
assertLabelsContains(studioPDB.getMetadata().getLabels(), "app.kubernetes.io/component=studio-ui",
"app.kubernetes.io/managed-by=apicurio-registry-operator",
"app.kubernetes.io/name=apicurio-registry");
assertLabelsContains(studioPDB.getSpec().getSelector().getMatchLabels(),
"app.kubernetes.io/component=studio-ui", "app.kubernetes.io/name=apicurio-registry",
"app.kubernetes.io/instance=" + registry.getMetadata().getName());
PodDisruptionBudgetStatus studioPdbStatus = studioPDB.getStatus();
Assertions.assertThat(studioPdbStatus.getExpectedPods()).isEqualTo(1);
Assertions.assertThat(studioPdbStatus.getDisruptionsAllowed()).isEqualTo(0);
}

private void assertLabelsContains(Map<String, String> labels, String... values) {
Assertions.assertThat(labels.entrySet().stream().map(l -> l.getKey() + "=" + l.getValue())
.collect(Collectors.toSet())).contains(values);
}
}
35 changes: 34 additions & 1 deletion operator/install/install.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,17 @@ spec:
automatically.'
type: string
type: object
podDisruptionBudget:
description: |
Configuration of a PodDisruptionBudget for the component.
properties:
enabled:
description: |
Whether a PodDisruptionBudget should be managed by the operator. Defaults to 'true'.
Set this to 'false' if you want to create your own custom PodDisruptionBudget.
type: boolean
type: object
podTemplateSpec:
description: |-
`PodTemplateSpec` describes the data a pod should have when created from a template.
Expand Down Expand Up @@ -3254,6 +3265,17 @@ spec:
IMPORTANT: If the Ingress already exists and the value becomes empty, the Ingress will be deleted.
type: string
type: object
podDisruptionBudget:
description: |
Configuration of a PodDisruptionBudget for the component.
properties:
enabled:
description: |
Whether a PodDisruptionBudget should be managed by the operator. Defaults to 'true'.
Set this to 'false' if you want to create your own custom PodDisruptionBudget.
type: boolean
type: object
podTemplateSpec:
description: |-
`PodTemplateSpec` describes the data a pod should have when created from a template.
Expand Down Expand Up @@ -6243,6 +6265,17 @@ spec:
IMPORTANT: If the Ingress already exists and the value becomes empty, the Ingress will be deleted.
type: string
type: object
podDisruptionBudget:
description: |
Configuration of a PodDisruptionBudget for the component.
properties:
enabled:
description: |
Whether a PodDisruptionBudget should be managed by the operator. Defaults to 'true'.
Set this to 'false' if you want to create your own custom PodDisruptionBudget.
type: boolean
type: object
podTemplateSpec:
description: |-
`PodTemplateSpec` describes the data a pod should have when created from a template.
Expand Down Expand Up @@ -9362,7 +9395,7 @@ spec:
value: quay.io/apicurio/apicurio-registry-ui:latest-snapshot
- name: STUDIO_UI_IMAGE
value: quay.io/apicurio/apicurio-studio-ui:latest-snapshot
image: quay.io/apicurio/apicurio-registry-operator:latest-snapshot
image: quay.io/apicurio/apicurio-registry-operator:3.0.7-snapshot
imagePullPolicy: Always
livenessProbe:
httpGet:
Expand Down
12 changes: 5 additions & 7 deletions operator/model/src/main/java/io/apicurio/registry/operator/api/v1/spec/ComponentSpec.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

@JsonDeserialize(using = None.class)
@JsonInclude(NON_NULL)
@JsonPropertyOrder({ "env", "ingress", "host", "podTemplateSpec", "manageDisruptionBudget" })
@JsonPropertyOrder({ "env", "ingress", "host", "podTemplateSpec", "podDisruptionBudget" })
@NoArgsConstructor(access = PROTECTED)
@AllArgsConstructor(access = PROTECTED)
@SuperBuilder(toBuilder = true)
Expand Down Expand Up @@ -100,15 +100,13 @@ public IngressSpec withIngress() {
}

/**
* Indicates whether to create a pod disruption budget
* Pod disruption budget config
*/
@JsonProperty("manageDisruptionBudget")
@JsonProperty("podDisruptionBudget")
@JsonPropertyDescription("""
Whether a PodDisruptionBudget should be managed by the operator. Defaults to 'true'.
Set this to 'false' if you want to create your own custom PodDisruptionBudget.
Configuration of a PodDisruptionBudget for the component.
""")
@JsonSetter(nulls = Nulls.SKIP)
private Boolean manageDisruptionBudget;
private PodDisruptionSpec podDisruptionBudget;

}
46 changes: 46 additions & 0 deletions ...ator/model/src/main/java/io/apicurio/registry/operator/api/v1/spec/PodDisruptionSpec.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package io.apicurio.registry.operator.api.v1.spec;

import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonPropertyDescription;
import com.fasterxml.jackson.annotation.JsonPropertyOrder;
import com.fasterxml.jackson.annotation.JsonSetter;
import com.fasterxml.jackson.annotation.Nulls;
import com.fasterxml.jackson.databind.JsonDeserializer.None;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import lombok.AllArgsConstructor;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.NoArgsConstructor;
import lombok.Setter;
import lombok.ToString;
import lombok.experimental.SuperBuilder;

import static com.fasterxml.jackson.annotation.JsonInclude.Include.NON_NULL;
import static lombok.AccessLevel.PRIVATE;

@JsonDeserialize(using = None.class)
@JsonInclude(NON_NULL)
@JsonPropertyOrder({ "enabled" })
@NoArgsConstructor
@AllArgsConstructor(access = PRIVATE)
@SuperBuilder(toBuilder = true)
@Getter
@Setter
@EqualsAndHashCode
@ToString
public class PodDisruptionSpec {

/**
* Indicates whether to create and manage a pod disruption budget
*/
@JsonProperty("enabled")
@JsonPropertyDescription("""
Whether a PodDisruptionBudget should be managed by the operator. Defaults to 'true'.
Set this to 'false' if you want to create your own custom PodDisruptionBudget.
""")
@JsonSetter(nulls = Nulls.SKIP)
private Boolean enabled;

}

0 comments on commit 8e7d3ca

Please sign in to comment.