Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

configmap based mechanism to override the admin/canary versions #643

Merged
merged 7 commits into from
Feb 2, 2022
Merged

configmap based mechanism to override the admin/canary versions #643

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e14c10e
configmap based mechanism to override the admin/canary versions
shawkins Jan 21, 2022
18d3904
using the configmap already in the bundle
shawkins Jan 25, 2022
2efc799
adding a test of the image override
shawkins Jan 25, 2022
27ef9a1
using more generic names, a basic parsing structure, and adding logging
shawkins Jan 26, 2022
ca20ed7
guarding against null strimzi
shawkins Jan 27, 2022
211edb7
modifying the structure again
shawkins Jan 28, 2022
1f40f3d
upping the memory available for tests
shawkins Feb 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
146 changes: 146 additions & 0 deletions operator/src/main/java/org/bf2/operator/managers/OperandOverrideManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
package org.bf2.operator.managers;

import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonProperty;
import io.fabric8.kubernetes.api.model.ConfigMap;
import io.fabric8.kubernetes.client.KubernetesClient;
import io.fabric8.kubernetes.client.informers.ResourceEventHandler;
import io.fabric8.kubernetes.client.utils.Serialization;
import io.quarkus.runtime.Startup;
import org.bf2.common.ResourceInformerFactory;
import org.eclipse.microprofile.config.inject.ConfigProperty;
import org.jboss.logging.Logger;

import javax.annotation.PostConstruct;
import javax.enterprise.context.ApplicationScoped;
import javax.inject.Inject;

import java.util.Map;
import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;

@Startup
@ApplicationScoped
public class OperandOverrideManager {

@JsonIgnoreProperties(ignoreUnknown = true)
public static class OperandOverride {
public String image;

public String getImage() {
return image;
}

public void setImage(String image) {
this.image = image;
}
}

@JsonIgnoreProperties(ignoreUnknown = true)
public static class OperandOverrides {
public OperandOverride canary = new OperandOverride();
@JsonProperty(value = "canary-init")
public OperandOverride canaryInit = new OperandOverride();
shawkins marked this conversation as resolved.
Show resolved Hide resolved
@JsonProperty(value = "admin-server")
public OperandOverride adminServer = new OperandOverride();
}

static final OperandOverrides EMPTY = new OperandOverrides();

public static final String OPERANDS_YAML = "fleetshard_operands.yaml";

private Map<String, OperandOverrides> overrides = new ConcurrentHashMap<>();

@ConfigProperty(name = "image.admin-api")
String adminApiImage;

@ConfigProperty(name = "image.canary")
String canaryImage;

@ConfigProperty(name = "image.canary-init")
String canaryInitImage;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we try to have a kind of same structure for the properties configured via env var or application.properties file instead of the fleetshard_operands.yaml" with overrides? I mean naming the above as admin-server.image, canary.image and canary.init.image ? Wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean naming the above as admin-server.image, canary.image and canary.init.image ? Wdyt?

In practical terms once this mechanism is utilized the old / default values should go unused. Turning the override into a properties file precludes easily using more structured parsing down the road (not that I'd like to overly worry about things that may not be needed). In the interest of time, I'd probably leave this as is at this point.


@Inject
KubernetesClient kubernetesClient;

@Inject
ResourceInformerFactory resourceInformerFactory;

@Inject
InformerManager informerManager;

@Inject
Logger log;

@PostConstruct
protected void onStart() {
this.resourceInformerFactory.create(ConfigMap.class,
this.kubernetesClient.configMaps().inAnyNamespace().withLabel("app", "strimzi"),
new ResourceEventHandler<ConfigMap>() {
@Override
public void onAdd(ConfigMap obj) {
updateOverrides(obj);
}

@Override
public void onDelete(ConfigMap obj, boolean deletedFinalStateUnknown) {
removeOverrides(obj);
}

@Override
public void onUpdate(ConfigMap oldObj, ConfigMap newObj) {
updateOverrides(newObj);
}
});
}

private OperandOverrides getOverrides(String strimzi) {
return overrides.getOrDefault(strimzi == null ? "" : strimzi, EMPTY);
}

public String getCanaryImage(String strimzi) {
return Optional.ofNullable(getOverrides(strimzi).canary.image).orElse(canaryImage);
}

public String getCanaryInitImage(String strimzi) {
return Optional.ofNullable(getOverrides(strimzi).canaryInit.image).orElse(canaryInitImage);
}

public String getAdminServerImage(String strimzi) {
return Optional.ofNullable(getOverrides(strimzi).adminServer.image).orElse(adminApiImage);
}

void updateOverrides(ConfigMap obj) {
String name = obj.getMetadata().getName();
if (name.startsWith(StrimziManager.STRIMZI_CLUSTER_OPERATOR)) {
String data = obj.getData().get(OPERANDS_YAML);
log.infof("Updating overrides for {} to {}", name, data);
boolean resync = false;
if (data == null) {
overrides.remove(name);
resync = true;
} else {
OperandOverrides operands = Serialization.unmarshal(data, OperandOverrides.class);
OperandOverrides old = overrides.put(name, operands);
resync = old == null || !Serialization.asYaml(old).equals(Serialization.asYaml(operands));
}
if (resync) {
informerManager.resyncManagedKafka();
}
}
}

void removeOverrides(ConfigMap obj) {
String name = obj.getMetadata().getName();
if (name.startsWith(StrimziManager.STRIMZI_CLUSTER_OPERATOR)) {
log.infof("removing overrides for {}", name);
overrides.remove(name);
informerManager.resyncManagedKafka();
}
}

void resetOverrides() {
this.overrides.clear();
}

}
3 changes: 2 additions & 1 deletion operator/src/main/java/org/bf2/operator/managers/StrimziManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
@ApplicationScoped
public class StrimziManager {

public static final String STRIMZI_CLUSTER_OPERATOR = "strimzi-cluster-operator";
public static final String STRIMZI_PAUSE_RECONCILE_ANNOTATION = "strimzi.io/pause-reconciliation";
public static final String STRIMZI_PAUSE_REASON_ANNOTATION = "managedkafka.bf2.org/pause-reason";

Expand Down Expand Up @@ -110,7 +111,7 @@ private void updateStatus() {
}

private boolean isStrimziDeployment(Deployment deployment) {
return deployment.getMetadata().getName().startsWith("strimzi-cluster-operator");
return deployment.getMetadata().getName().startsWith(STRIMZI_CLUSTER_OPERATOR);
}
});
}
Expand Down
9 changes: 5 additions & 4 deletions operator/src/main/java/org/bf2/operator/operands/AdminServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.bf2.common.OperandUtils;
import org.bf2.operator.managers.ImagePullSecretManager;
import org.bf2.operator.managers.IngressControllerManager;
import org.bf2.operator.managers.OperandOverrideManager;
import org.bf2.operator.managers.SecuritySecretManager;
import org.bf2.operator.resources.v1alpha1.ManagedKafka;
import org.bf2.operator.resources.v1alpha1.ManagedKafkaAuthenticationOAuth;
Expand Down Expand Up @@ -81,9 +82,6 @@ public class AdminServer extends AbstractAdminServer {
@Inject
Logger log;

@ConfigProperty(name = "image.admin-api")
String adminApiImage;

@ConfigProperty(name = "adminserver.cors.allowlist")
Optional<String> corsAllowList;

Expand All @@ -101,6 +99,9 @@ public class AdminServer extends AbstractAdminServer {
@Inject
protected Instance<IngressControllerManager> ingressControllerManagerInstance;

@Inject
protected OperandOverrideManager overrideManager;

void onStart(@Observes StartupEvent ev) {
if (kubernetesClient.isAdaptable(OpenShiftClient.class)) {
openShiftClient = kubernetesClient.adapt(OpenShiftClient.class);
Expand Down Expand Up @@ -257,7 +258,7 @@ protected Route routeFrom(ManagedKafka managedKafka, Route current) {
protected List<Container> buildContainers(ManagedKafka managedKafka) {
Container container = new ContainerBuilder()
.withName("admin-server")
.withImage(adminApiImage)
.withImage(overrideManager.getAdminServerImage(managedKafka.getSpec().getVersions().getStrimzi()))
.withEnv(buildEnvVar(managedKafka))
.withPorts(buildContainerPorts(managedKafka))
.withResources(buildResources())
Expand Down
14 changes: 6 additions & 8 deletions operator/src/main/java/org/bf2/operator/operands/Canary.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.bf2.common.OperandUtils;
import org.bf2.operator.managers.ImagePullSecretManager;
import org.bf2.operator.managers.IngressControllerManager;
import org.bf2.operator.managers.OperandOverrideManager;
import org.bf2.operator.managers.SecuritySecretManager;
import org.bf2.operator.resources.v1alpha1.ManagedKafka;
import org.bf2.operator.resources.v1alpha1.ServiceAccount;
Expand Down Expand Up @@ -58,12 +59,6 @@ public class Canary extends AbstractCanary {
private static final String METRICS_PORT_NAME = "metrics";
private static final IntOrString METRICS_PORT_TARGET = new IntOrString(METRICS_PORT_NAME);

@ConfigProperty(name = "image.canary")
String canaryImage;

@ConfigProperty(name = "image.canary-init")
String canaryInitImage;

@ConfigProperty(name = "managedkafka.canary.producer-latency-buckets")
String producerLatencyBuckets;

Expand Down Expand Up @@ -91,6 +86,9 @@ public class Canary extends AbstractCanary {
@Inject
protected KafkaInstanceConfiguration config;

@Inject
protected OperandOverrideManager overrideManager;

@Override
public Deployment deploymentFrom(ManagedKafka managedKafka, Deployment current) {
String canaryName = canaryName(managedKafka);
Expand Down Expand Up @@ -192,7 +190,7 @@ private List<Volume> buildVolumes(ManagedKafka managedKafka) {
protected Container buildInitContainer(ManagedKafka managedKafka, Deployment current) {
return new ContainerBuilder()
.withName("init")
.withImage(canaryInitImage)
.withImage(overrideManager.getCanaryInitImage(managedKafka.getSpec().getVersions().getStrimzi()))
.withEnv(buildInitEnvVar(managedKafka))
.withResources(buildResources())
.withCommand("/opt/strimzi-canary-tool/canary-dns-init.sh")
Expand All @@ -208,7 +206,7 @@ protected boolean hasClusterSpecificBootstrapDomain(ManagedKafka managedKafka) {
protected List<Container> buildContainers(ManagedKafka managedKafka, Deployment current) {
Container container = new ContainerBuilder()
.withName("canary")
.withImage(canaryImage)
.withImage(overrideManager.getCanaryImage(managedKafka.getSpec().getVersions().getStrimzi()))
.withEnv(buildEnvVar(managedKafka, current))
.withPorts(buildContainerPorts())
.withResources(buildResources())
Expand Down
56 changes: 56 additions & 0 deletions operator/src/test/java/org/bf2/operator/managers/OperandOverrideManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package org.bf2.operator.managers;

import io.fabric8.kubernetes.api.model.ConfigMapBuilder;
import io.fabric8.kubernetes.client.KubernetesClient;
import io.quarkus.test.common.QuarkusTestResource;
import io.quarkus.test.junit.QuarkusTest;
import io.quarkus.test.junit.TestProfile;
import io.quarkus.test.kubernetes.client.KubernetesServerTestResource;
import org.bf2.operator.MockProfile;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;

import javax.inject.Inject;

import java.util.Collections;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotEquals;

@QuarkusTestResource(KubernetesServerTestResource.class)
@TestProfile(MockProfile.class)
@QuarkusTest
public class OperandOverrideManagerTest {

@Inject
KubernetesClient client;

@Inject
OperandOverrideManager overrideManager;

@AfterEach
public void cleanup() {
overrideManager.resetOverrides();
}

@Test
void testImageOverride() {
String versionString = "strimzi-cluster-operator-0.26-1";
String defaultVersion = overrideManager.getCanaryImage(versionString);

overrideManager.updateOverrides(new ConfigMapBuilder().withNewMetadata()
.withName(versionString)
.endMetadata()
.withData(Collections.singletonMap(OperandOverrideManager.OPERANDS_YAML,
"canary: \n"
+ " image: something\n"
+ " notused: value\n"))
.build());

String override = overrideManager.getCanaryImage(versionString);

assertEquals("something", override);
assertNotEquals(defaultVersion, override);
}

}