-
Notifications
You must be signed in to change notification settings - Fork 20
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.
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
Changes from 6 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 18d3904
using the configmap already in the bundle
shawkins 2efc799
adding a test of the image override
shawkins 27ef9a1
using more generic names, a basic parsing structure, and adding logging
shawkins ca20ed7
guarding against null strimzi
shawkins 211edb7
modifying the structure again
shawkins 1f40f3d
upping the memory available for tests
shawkins File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
149 changes: 149 additions & 0 deletions
149
operator/src/main/java/org/bf2/operator/managers/OperandOverrideManager.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,149 @@ | ||
| 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 Canary extends OperandOverride { | ||
| public OperandOverride init = new OperandOverride(); | ||
| } | ||
|
|
||
| @JsonIgnoreProperties(ignoreUnknown = true) | ||
| public static class OperandOverrides { | ||
| public Canary canary = new Canary(); | ||
| @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; | ||
|
|
||
| @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).canary.init.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(); | ||
| } | ||
|
|
||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
62 changes: 62 additions & 0 deletions
62
operator/src/test/java/org/bf2/operator/managers/OperandOverrideManagerTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| 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" | ||
| + " init: \n" | ||
| + " image: somethingelse\n")) | ||
| .build()); | ||
|
|
||
| String override = overrideManager.getCanaryImage(versionString); | ||
|
|
||
| assertEquals("something", override); | ||
| assertNotEquals(defaultVersion, override); | ||
|
|
||
| String initOverride = overrideManager.getCanaryInitImage(versionString); | ||
|
|
||
| assertEquals("somethingelse", initOverride); | ||
| } | ||
|
|
||
| } |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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 asadmin-server.image,canary.imageandcanary.init.image? Wdyt?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.