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

fix: add proper default install mode derived from watched namespaces #841

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: add proper default install mode derived from watched namespaces #841

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 3 additions & 2 deletions ...eployment/src/main/java/io/quarkiverse/operatorsdk/bundle/deployment/BundleGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import io.quarkiverse.operatorsdk.common.ReconcilerAugmentedClassInfo;
import io.quarkiverse.operatorsdk.runtime.BuildTimeOperatorConfiguration;
import io.quarkiverse.operatorsdk.runtime.CRDInfo;
import io.quarkiverse.operatorsdk.runtime.QuarkusControllerConfiguration;
import io.quarkiverse.operatorsdk.runtime.Version;
import io.quarkus.container.util.PathsUtil;

Expand Down Expand Up @@ -49,15 +50,15 @@ private BundleGenerator() {
public static List<ManifestsBuilder> prepareGeneration(BundleGenerationConfiguration bundleConfiguration,
BuildTimeOperatorConfiguration operatorConfiguration, Version version,
Map<CSVMetadataHolder, List<ReconcilerAugmentedClassInfo>> csvGroups, Map<String, CRDInfo> crds,
Path outputDirectory, String deploymentName) {
Path outputDirectory, String deploymentName, Map<String, QuarkusControllerConfiguration<?>> controllerConfigs) {
List<ManifestsBuilder> builders = new ArrayList<>();
for (Map.Entry<CSVMetadataHolder, List<ReconcilerAugmentedClassInfo>> entry : csvGroups.entrySet()) {
final var csvMetadata = entry.getKey();
final var labels = generateBundleLabels(csvMetadata, bundleConfiguration, version);

final var mainSourcesRoot = PathsUtil.findMainSourcesRoot(outputDirectory);
final var csvBuilder = new CsvManifestsBuilder(csvMetadata, operatorConfiguration, entry.getValue(),
mainSourcesRoot != null ? mainSourcesRoot.getKey() : null, deploymentName);
mainSourcesRoot != null ? mainSourcesRoot.getKey() : null, deploymentName, controllerConfigs);
builders.add(csvBuilder);
builders.add(new AnnotationsManifestsBuilder(csvMetadata, labels));
builders.add(new BundleDockerfileManifestsBuilder(csvMetadata, labels));
Expand Down
7 changes: 5 additions & 2 deletions ...eployment/src/main/java/io/quarkiverse/operatorsdk/bundle/deployment/BundleProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import io.quarkiverse.operatorsdk.bundle.runtime.BundleGenerationConfiguration;
import io.quarkiverse.operatorsdk.bundle.runtime.CSVMetadataHolder;
import io.quarkiverse.operatorsdk.common.*;
import io.quarkiverse.operatorsdk.deployment.ControllerConfigurationsBuildItem;
import io.quarkiverse.operatorsdk.deployment.GeneratedCRDInfoBuildItem;
import io.quarkiverse.operatorsdk.deployment.VersionBuildItem;
import io.quarkiverse.operatorsdk.runtime.BuildTimeOperatorConfiguration;
Expand Down Expand Up @@ -193,7 +194,8 @@ void generateBundle(ApplicationInfoBuildItem configuration,
BuildProducer<GeneratedBundleBuildItem> doneGeneratingCSV,
GeneratedCRDInfoBuildItem generatedCustomResourcesDefinitions,
@SuppressWarnings("OptionalUsedAsFieldOrParameterType") Optional<DeserializedKubernetesResourcesBuildItem> maybeGeneratedKubeResources,
BuildProducer<GeneratedFileSystemResourceBuildItem> generatedCSVs) {
BuildProducer<GeneratedFileSystemResourceBuildItem> generatedCSVs,
ControllerConfigurationsBuildItem controllerConfigurationsBuildItem) {
final var crds = generatedCustomResourcesDefinitions.getCRDGenerationInfo().getCrds()
.values().stream()
.flatMap(entry -> entry.values().stream())
Expand Down Expand Up @@ -243,7 +245,8 @@ void generateBundle(ApplicationInfoBuildItem configuration,
final var deploymentName = ResourceNameUtil.getResourceName(kubernetesConfig, configuration);
final var generated = BundleGenerator.prepareGeneration(bundleConfiguration, operatorConfiguration,
versionBuildItem.getVersion(),
csvMetadata.getCsvGroups(), crds, outputTarget.getOutputDirectory(), deploymentName);
csvMetadata.getCsvGroups(), crds, outputTarget.getOutputDirectory(), deploymentName,
controllerConfigurationsBuildItem.getControllerConfigs());
generated.forEach(manifestBuilder -> {
final var fileName = manifestBuilder.getFileName();
try {
Expand Down
39 changes: 29 additions & 10 deletions .../main/java/io/quarkiverse/operatorsdk/bundle/deployment/builders/CsvManifestsBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,13 @@
import io.quarkiverse.operatorsdk.bundle.runtime.CSVMetadataHolder.RequiredCRD;
import io.quarkiverse.operatorsdk.common.*;
import io.quarkiverse.operatorsdk.runtime.BuildTimeOperatorConfiguration;
import io.quarkiverse.operatorsdk.runtime.QuarkusControllerConfiguration;

public class CsvManifestsBuilder extends ManifestsBuilder {

private static final Logger log = Logger.getLogger(CsvManifestsBuilder.class);

private static final String DEFAULT_INSTALL_MODE = "AllNamespaces";
private static final String ALL_NAMESPACES = "AllNamespaces";
private static final String DEPLOYMENT = "deployment";
private static final String SERVICE_ACCOUNT_KIND = "ServiceAccount";
private static final String CLUSTER_ROLE_KIND = "ClusterRole";
Expand All @@ -37,6 +38,9 @@ public class CsvManifestsBuilder extends ManifestsBuilder {
private static final Logger LOGGER = Logger.getLogger(CsvManifestsBuilder.class.getName());
private static final String IMAGE_PNG = "image/png";
public static final String OLM_TARGET_NAMESPACES = "metadata.annotations['olm.targetNamespaces']";
public static final String OWN_NAMESPACE = "OwnNamespace";
public static final String SINGLE_NAMESPACE = "SingleNamespace";
public static final String MULTI_NAMESPACE = "MultiNamespace";
private ClusterServiceVersionBuilder csvBuilder;
private final Set<CRDDescription> ownedCRs = new HashSet<>();
private final Set<CRDDescription> requiredCRs = new HashSet<>();
Expand All @@ -46,7 +50,7 @@ public class CsvManifestsBuilder extends ManifestsBuilder {

public CsvManifestsBuilder(CSVMetadataHolder metadata, BuildTimeOperatorConfiguration operatorConfiguration,
List<ReconcilerAugmentedClassInfo> controllers,
Path mainSourcesRoot, String deploymentName) {
Path mainSourcesRoot, String deploymentName, Map<String, QuarkusControllerConfiguration<?>> controllerConfigs) {
super(metadata);
this.deploymentName = deploymentName;
this.controllers = controllers;
Expand Down Expand Up @@ -135,14 +139,6 @@ public CsvManifestsBuilder(CSVMetadataHolder metadata, BuildTimeOperatorConfigur
}
}

if (metadata.installModes == null || metadata.installModes.length == 0) {
csvSpecBuilder.addNewInstallMode(true, DEFAULT_INSTALL_MODE);
} else {
for (CSVMetadataHolder.InstallMode installMode : metadata.installModes) {
csvSpecBuilder.addNewInstallMode(installMode.supported, installMode.type);
}
}

// add owned and required CRD, also collect them
final var nativeApis = new ArrayList<GroupVersionKind>();
controllers.forEach(raci -> {
Expand Down Expand Up @@ -181,6 +177,29 @@ public CsvManifestsBuilder(CSVMetadataHolder metadata, BuildTimeOperatorConfigur
}
});
}

// deal with install modes
// use watched namespaces information for default install mode
// fixme: multiple, incompatible controller configurations in the same bundle will result in inconsistent runs
final var config = controllerConfigs.get(raci.nameOrFailIfUnset());
if (config.watchAllNamespaces()) {
csvSpecBuilder.withInstallModes(new InstallMode(true, ALL_NAMESPACES));
} else if (config.watchCurrentNamespace()) {
csvSpecBuilder.withInstallModes(new InstallMode(true, OWN_NAMESPACE));
} else {
final var namespaces = config.getNamespaces();
if (namespaces.size() == 1) {
csvSpecBuilder.withInstallModes(new InstallMode(true, SINGLE_NAMESPACE));
} else {
csvSpecBuilder.withInstallModes(new InstallMode(true, MULTI_NAMESPACE));
}
}
// then process metadata
if (metadata.installModes != null) {
for (CSVMetadataHolder.InstallMode installMode : metadata.installModes) {
csvSpecBuilder.addNewInstallMode(installMode.supported, installMode.type);
}
}
});

// add required CRDs from CSV metadata
Expand Down
14 changes: 12 additions & 2 deletions ...ployment/src/test/java/io/quarkiverse/operatorsdk/bundle/MultipleOperatorsBundleTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
package io.quarkiverse.operatorsdk.bundle;

import static io.quarkiverse.operatorsdk.bundle.Utils.*;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.*;

import java.io.IOException;

Expand Down Expand Up @@ -49,6 +48,13 @@ public void shouldWriteBundleForTheOperators() throws IOException {
assertEquals(FirstReconciler.REPLACES, csv.getSpec().getReplaces());
var bundleMeta = getAnnotationsFor(bundle, "first-operator");
assertEquals(BUNDLE_PACKAGE, bundleMeta.getAnnotations().get("operators.operatorframework.io.bundle.package.v1"));
assertEquals(2, csv.getSpec().getInstallModes().size());
var installMode = csv.getSpec().getInstallModes().get(0);
assertEquals("AllNamespaces", installMode.getType());
Copy link
Member

Choose a reason for hiding this comment

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

we could use the constants here too? I don't think this will change so not blocking this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm always debating whether or not to use constants in the tests, especially when the tests are checking compatibility with an external service or spec. In this case, if we used a constant, and the constant value changed but not the external spec, then the test would still pass even though it would now output incorrect values.

assertTrue(installMode.getSupported());
installMode = csv.getSpec().getInstallModes().get(1);
assertEquals("MultiNamespace", installMode.getType());
assertFalse(installMode.getSupported());

checkBundleFor(bundle, "second-operator", Second.class);
csv = getCSVFor(bundle, "second-operator");
Expand All @@ -59,6 +65,10 @@ public void shouldWriteBundleForTheOperators() throws IOException {
.addToResources(SecondReconciler.RBAC_RULE_RES)
.addToVerbs(SecondReconciler.RBAC_RULE_VERBS)
.build()));
assertEquals(1, csv.getSpec().getInstallModes().size());
installMode = csv.getSpec().getInstallModes().get(0);
assertEquals("SingleNamespace", installMode.getType());
assertTrue(installMode.getSupported());

checkBundleFor(bundle, "third-operator", Third.class);
// also check that external CRD is present
Expand Down
2 changes: 1 addition & 1 deletion ...r/deployment/src/test/java/io/quarkiverse/operatorsdk/bundle/sources/FirstReconciler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;
import io.quarkiverse.operatorsdk.annotations.CSVMetadata;

@CSVMetadata(bundleName = "first-operator", version = FirstReconciler.VERSION)
@CSVMetadata(bundleName = "first-operator", version = FirstReconciler.VERSION, installModes = @CSVMetadata.InstallMode(type = "MultiNamespace", supported = false))
public class FirstReconciler implements Reconciler<First> {

public static final String VERSION = "first-version";
Expand Down
Loading