Skip to content

Commit

Permalink
refactor(operator): address code review
Browse files Browse the repository at this point in the history
  • Loading branch information
@jsenko
jsenko committed Oct 24, 2024
1 parent 7a18bdb commit 495b4d6
Show file tree
Hide file tree
Showing 8 changed files with 90 additions and 122 deletions.
9 changes: 5 additions & 4 deletions app/src/test/java/io/apicurio/registry/noprofile/rest/v3/VersionModifiedByOnTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,12 @@ public void testVersionMetaDataWithModified() throws Exception {
String artifactId = TestUtils.generateArtifactId();

createArtifact(groupId, artifactId, ArtifactType.OPENAPI, artifactContent,
ContentTypes.APPLICATION_JSON, (ca) -> {
ca.getFirstVersion().setVersion("1.0");
});
ContentTypes.APPLICATION_JSON, (ca) -> {
ca.getFirstVersion().setVersion("1.0");
});

VersionMetaData vmd = clientV3.groups().byGroupId(groupId).artifacts().byArtifactId(artifactId).versions().byVersionExpression("1.0").get();
VersionMetaData vmd = clientV3.groups().byGroupId(groupId).artifacts().byArtifactId(artifactId)
.versions().byVersionExpression("1.0").get();
Assertions.assertNotNull(vmd);
Assertions.assertNotNull(vmd.getModifiedOn());
}
Expand Down
167 changes: 67 additions & 100 deletions ...ator/controller/src/main/java/io/apicurio/registry/operator/resource/ResourceFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,10 @@

import java.nio.charset.Charset;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import static io.apicurio.registry.operator.resource.app.AppDeploymentResource.getContainer;
import static io.apicurio.registry.operator.resource.app.AppDeploymentResource.getContainerFromPodTemplateSpec;
import static io.apicurio.registry.operator.utils.Mapper.YAML_MAPPER;
import static io.apicurio.registry.operator.utils.Utils.isBlank;
import static io.apicurio.registry.operator.utils.Utils.mergeNotOverride;
Expand All @@ -39,157 +38,125 @@ public class ResourceFactory {

public Deployment getDefaultAppDeployment(ApicurioRegistry3 primary) {
var r = new Deployment();

// Metadata
if (r.getMetadata() == null) {
r.setMetadata(new ObjectMeta());
}
if (r.getMetadata().getLabels() == null) {
r.getMetadata().setLabels(new HashMap<>());
}
r.setMetadata(new ObjectMeta());
r.getMetadata().setNamespace(primary.getMetadata().getNamespace());
r.getMetadata().setName(
primary.getMetadata().getName() + "-" + COMPONENT_APP + "-" + RESOURCE_TYPE_DEPLOYMENT);
addDefaultLabels(r.getMetadata().getLabels(), primary, COMPONENT_APP);

// Spec
r.setSpec(new DeploymentSpec());
r.getSpec().setReplicas(1);
r.getSpec().setSelector(new LabelSelector());
addSelectorLabels(r.getSpec().getSelector().getMatchLabels(), primary, COMPONENT_APP);
if (primary.getSpec().getApp().getPodTemplateSpec() != null) {
r.getSpec().setTemplate(primary.getSpec().getApp().getPodTemplateSpec());
} else {
r.getSpec().setTemplate(new PodTemplateSpec());
}
if (r.getSpec().getTemplate().getMetadata() == null) {
r.getSpec().getTemplate().setMetadata(new ObjectMeta());
}
mergeDeploymentPodTemplateSpec(
// spotless:off
r.getSpec().getTemplate(),
APP_CONTAINER_NAME,
"quay.io/apicurio/apicurio-registry:latest-snapshot",
List.of(new ContainerPortBuilder().withName("http").withProtocol("TCP").withContainerPort(8080).build()),
new ProbeBuilder().withHttpGet(new HTTPGetActionBuilder().withPath("/health/ready").withPort(new IntOrString(8080)).withScheme("HTTP").build()).build(),
new ProbeBuilder().withHttpGet(new HTTPGetActionBuilder().withPath("/health/live").withPort(new IntOrString(8080)).withScheme("HTTP").build()).build(),
Map.of("cpu", new Quantity("500m"), "memory", new Quantity("512Mi")),
Map.of("cpu", new Quantity("1"), "memory", new Quantity("1Gi"))
// spotless:on
);
addDefaultLabels(r.getMetadata().getLabels(), primary, COMPONENT_APP);
addSelectorLabels(r.getSpec().getSelector().getMatchLabels(), primary, COMPONENT_APP);
addDefaultLabels(r.getSpec().getTemplate().getMetadata().getLabels(), primary, COMPONENT_APP);
var c = getContainer(r.getSpec().getTemplate(), APP_CONTAINER_NAME);
if (c == null) {
if (r.getSpec().getTemplate().getSpec() == null) {
r.getSpec().getTemplate().setSpec(new PodSpec());
}
c = new Container();
c.setName(APP_CONTAINER_NAME);
if (r.getSpec().getTemplate().getSpec().getContainers() == null) {
r.getSpec().getTemplate().getSpec().setContainers(new ArrayList<>());
}
r.getSpec().getTemplate().getSpec().getContainers().add(c);
}
if (isBlank(c.getImage())) {
c.setImage("quay.io/apicurio/apicurio-registry:latest-snapshot");
}
if (c.getEnv() != null && !c.getEnv().isEmpty()) {
throw new OperatorException("""
Field spec.(app/ui).podTemplateSpec.spec.containers[name = %s].env must be empty. \
Use spec.(app/ui).env to configure environment variables."""
.formatted(APP_CONTAINER_NAME));
}
if (c.getPorts() == null) {
c.setPorts(new ArrayList<>());
}
mergeNotOverride(c.getPorts(), List.of(new ContainerPortBuilder().withName("http").withProtocol("TCP")
.withContainerPort(8080).build()), ContainerPort::getName);
if (c.getReadinessProbe() == null) {
c.setReadinessProbe(
new ProbeBuilder().withHttpGet(new HTTPGetActionBuilder().withPath("/health/ready")
.withPort(new IntOrString(8080)).withScheme("HTTP").build()).build());
}
if (c.getLivenessProbe() == null) {
c.setLivenessProbe(
new ProbeBuilder().withHttpGet(new HTTPGetActionBuilder().withPath("/health/live")
.withPort(new IntOrString(8080)).withScheme("HTTP").build()).build());
}
if (c.getResources() == null) {
c.setResources(new ResourceRequirements());
}
if (c.getResources().getRequests() == null || c.getResources().getRequests().isEmpty()) {
c.getResources()
.setRequests(Map.of("cpu", new Quantity("500m"), "memory", new Quantity("512Mi")));
}
if (c.getResources().getLimits() == null || c.getResources().getLimits().isEmpty()) {
c.getResources().setLimits(Map.of("cpu", new Quantity("1"), "memory", new Quantity("1Gi")));
}

return r;
}

public Deployment getDefaultUIDeployment(ApicurioRegistry3 primary) {
var r = new Deployment();

// Metadata
if (r.getMetadata() == null) {
r.setMetadata(new ObjectMeta());
}
r.setMetadata(new ObjectMeta());
r.getMetadata().setNamespace(primary.getMetadata().getNamespace());
r.getMetadata().setName(
primary.getMetadata().getName() + "-" + COMPONENT_UI + "-" + RESOURCE_TYPE_DEPLOYMENT);
addDefaultLabels(r.getMetadata().getLabels(), primary, COMPONENT_UI);

// Spec
r.setSpec(new DeploymentSpec());
r.getSpec().setReplicas(1);
r.getSpec().setSelector(new LabelSelector());
addSelectorLabels(r.getSpec().getSelector().getMatchLabels(), primary, COMPONENT_UI);
if (primary.getSpec().getUi().getPodTemplateSpec() != null) {
r.getSpec().setTemplate(primary.getSpec().getUi().getPodTemplateSpec());
} else {
r.getSpec().setTemplate(new PodTemplateSpec());
}
if (r.getSpec().getTemplate().getMetadata() == null) {
r.getSpec().getTemplate().setMetadata(new ObjectMeta());
}
mergeDeploymentPodTemplateSpec(
// spotless:off
r.getSpec().getTemplate(),
UI_CONTAINER_NAME,
"quay.io/apicurio/apicurio-registry-ui:latest-snapshot",
List.of(new ContainerPortBuilder().withName("http").withProtocol("TCP").withContainerPort(8080).build()),
new ProbeBuilder().withHttpGet(new HTTPGetActionBuilder().withPath("/config.js").withPort(new IntOrString(8080)).withScheme("HTTP").build()).build(),
new ProbeBuilder().withHttpGet(new HTTPGetActionBuilder().withPath("/config.js").withPort(new IntOrString(8080)).withScheme("HTTP").build()).build(),
Map.of("cpu", new Quantity("100m"), "memory", new Quantity("256Mi")),
Map.of("cpu", new Quantity("200m"), "memory", new Quantity("512Mi"))
// spotless:on
);
addDefaultLabels(r.getMetadata().getLabels(), primary, COMPONENT_UI);
addSelectorLabels(r.getSpec().getSelector().getMatchLabels(), primary, COMPONENT_UI);
addDefaultLabels(r.getSpec().getTemplate().getMetadata().getLabels(), primary, COMPONENT_UI);
var c = getContainer(r.getSpec().getTemplate(), UI_CONTAINER_NAME);
return r;
}

/**
* Merge default values for a Deployment into the target PTS (from spec).
*/
private static void mergeDeploymentPodTemplateSpec(
// spotless:off
PodTemplateSpec target,
String containerName,
String image,
List<ContainerPort> ports,
Probe readinessProbe,
Probe livenessProbe,
Map<String, Quantity> requests,
Map<String, Quantity> limits
// spotless:on
) {
if (target.getMetadata() == null) {
target.setMetadata(new ObjectMeta());
}
var c = getContainerFromPodTemplateSpec(target, containerName);
if (c == null) {
if (r.getSpec().getTemplate().getSpec() == null) {
r.getSpec().getTemplate().setSpec(new PodSpec());
if (target.getSpec() == null) {
target.setSpec(new PodSpec());
}
c = new Container();
c.setName(UI_CONTAINER_NAME);
if (r.getSpec().getTemplate().getSpec().getContainers() == null) {
r.getSpec().getTemplate().getSpec().setContainers(new ArrayList<>());
c.setName(containerName);
if (target.getSpec().getContainers() == null) {
target.getSpec().setContainers(new ArrayList<>());
}
r.getSpec().getTemplate().getSpec().getContainers().add(c);
target.getSpec().getContainers().add(c);
}
if (isBlank(c.getImage())) {
c.setImage("quay.io/apicurio/apicurio-registry-ui:latest-snapshot");
c.setImage(image);
}
if (c.getEnv() != null && !c.getEnv().isEmpty()) {
throw new OperatorException("""
Field spec.(app/ui).podTemplateSpec.spec.containers[name = %s].env must be empty. \
Use spec.(app/ui).env to configure environment variables."""
.formatted(UI_CONTAINER_NAME));
Field spec.(app/ui).podTemplateSpec.spec.containers[name = %s].env must be empty. \
Use spec.(app/ui).env to configure environment variables.""".formatted(containerName));
}
if (c.getPorts() == null) {
c.setPorts(new ArrayList<>());
}
mergeNotOverride(c.getPorts(), List.of(new ContainerPortBuilder().withName("http").withProtocol("TCP")
.withContainerPort(8080).build()), ContainerPort::getName);
mergeNotOverride(c.getPorts(), ports, ContainerPort::getName);
if (c.getReadinessProbe() == null) {
c.setReadinessProbe(
new ProbeBuilder().withHttpGet(new HTTPGetActionBuilder().withPath("/config.js")
.withPort(new IntOrString(8080)).withScheme("HTTP").build()).build());
c.setReadinessProbe(readinessProbe);
}
if (c.getLivenessProbe() == null) {
c.setLivenessProbe(
new ProbeBuilder().withHttpGet(new HTTPGetActionBuilder().withPath("/config.js")
.withPort(new IntOrString(8080)).withScheme("HTTP").build()).build());
c.setLivenessProbe(livenessProbe);
}
if (c.getResources() == null) {
c.setResources(new ResourceRequirements());
}
if (c.getResources().getRequests() == null || c.getResources().getRequests().isEmpty()) {
c.getResources()
.setRequests(Map.of("cpu", new Quantity("100m"), "memory", new Quantity("256Mi")));
c.getResources().setRequests(requests);
}
if (c.getResources().getLimits() == null || c.getResources().getLimits().isEmpty()) {
c.getResources().setLimits(Map.of("cpu", new Quantity("200m"), "memory", new Quantity("512Mi")));
c.getResources().setLimits(limits);
}

return r;
}

public Service getDefaultAppService(ApicurioRegistry3 primary) {
Expand Down
8 changes: 4 additions & 4 deletions ...oller/src/main/java/io/apicurio/registry/operator/resource/app/AppDeploymentResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ protected Deployment desired(ApicurioRegistry3 primary, Context<ApicurioRegistry

configureSqlDatasource(envVars, primary.getSpec().getApp().getSql());

var container = getContainer(d, APP_CONTAINER_NAME);
var container = getContainerFromDeployment(d, APP_CONTAINER_NAME);
container.setEnv(envVars.values().stream().toList());

log.debug("Desired {} is {}", APP_DEPLOYMENT_KEY.getId(), toYAML(d));
Expand Down Expand Up @@ -105,12 +105,12 @@ public static void addEnvVar(Map<String, EnvVar> map, EnvVar envVar) {
*
* @throws OperatorException if container was not found
*/
public static Container getContainer(Deployment d, String name) {
public static Container getContainerFromDeployment(Deployment d, String name) {
requireNonNull(d);
requireNonNull(name);
log.debug("Getting container {} in Deployment {}", name, ResourceID.fromResource(d));
if (d.getSpec() != null & d.getSpec().getTemplate() != null) {
var c = getContainer(d.getSpec().getTemplate(), name);
var c = getContainerFromPodTemplateSpec(d.getSpec().getTemplate(), name);
if (c != null) {
return c;
}
Expand All @@ -124,7 +124,7 @@ public static Container getContainer(Deployment d, String name) {
*
* @return null when container was not found
*/
public static Container getContainer(PodTemplateSpec pts, String name) {
public static Container getContainerFromPodTemplateSpec(PodTemplateSpec pts, String name) {
requireNonNull(pts);
requireNonNull(name);
if (pts.getSpec() != null && pts.getSpec().getContainers() != null) {
Expand Down
4 changes: 2 additions & 2 deletions ...troller/src/main/java/io/apicurio/registry/operator/resource/ui/UIDeploymentResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import static io.apicurio.registry.operator.resource.ResourceFactory.UI_CONTAINER_NAME;
import static io.apicurio.registry.operator.resource.ResourceKey.*;
import static io.apicurio.registry.operator.resource.app.AppDeploymentResource.addEnvVar;
import static io.apicurio.registry.operator.resource.app.AppDeploymentResource.getContainer;
import static io.apicurio.registry.operator.resource.app.AppDeploymentResource.getContainerFromDeployment;
import static io.apicurio.registry.operator.utils.IngressUtils.withIngressRule;
import static io.apicurio.registry.operator.utils.Mapper.toYAML;

Expand Down Expand Up @@ -57,7 +57,7 @@ protected Deployment desired(ApicurioRegistry3 primary, Context<ApicurioRegistry
}));
});

var container = getContainer(d, UI_CONTAINER_NAME);
var container = getContainerFromDeployment(d, UI_CONTAINER_NAME);
container.setEnv(envVars.values().stream().toList());

log.debug("Desired {} is {}", UI_DEPLOYMENT_KEY.getId(), toYAML(d));
Expand Down
Loading

0 comments on commit 495b4d6

Please sign in to comment.