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.

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: record version with CRD name to properly output multiple versions #973

Merged
merged 1 commit into from
Oct 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import io.javaoperatorsdk.operator.api.reconciler.Ignore;
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.GenericKubernetesDependentResource;
import io.quarkiverse.operatorsdk.annotations.AdditionalRBACRoleRefs;
import io.quarkiverse.operatorsdk.annotations.AdditionalRBACRules;
import io.quarkiverse.operatorsdk.annotations.RBACCRoleRef;
Expand All @@ -25,6 +26,8 @@ private Constants() {
public static final DotName HAS_METADATA = DotName.createSimple(HasMetadata.class.getName());
public static final DotName CONTROLLER_CONFIGURATION = DotName.createSimple(ControllerConfiguration.class.getName());
public static final DotName DEPENDENT_RESOURCE = DotName.createSimple(DependentResource.class.getName());
public static final DotName GENERIC_KUBERNETES_DEPENDENT_RESOURCE = DotName
.createSimple(GenericKubernetesDependentResource.class.getName());
public static final DotName CONFIGURED = DotName.createSimple(Configured.class.getName());
public static final DotName ANNOTATION_CONFIGURABLE = DotName.createSimple(AnnotationConfigurable.class.getName());
public static final DotName OBJECT = DotName.createSimple(Object.class.getName());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,12 @@ protected CustomResourceAugmentedClassInfo(ClassInfo classInfo, String associate

@Override
protected boolean doKeep(IndexView index, Logger log, Map<String, Object> context) {

// only keep the information if the associated CRD hasn't already been generated
final var fullName = fullResourceName();
return Optional.ofNullable(context.get(EXISTING_CRDS_KEY))
.map(value -> {
@SuppressWarnings("unchecked")
Set<String> generated = (Set<String>) value;
return !generated.contains(fullName);
return !generated.contains(id());
})
.orElse(true);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,24 +16,26 @@ public class DependentResourceAugmentedClassInfo extends ResourceAssociatedAugme
private final AnnotationInstance dependentAnnotationFromController;

public DependentResourceAugmentedClassInfo(ClassInfo classInfo) {
this(classInfo, null);
this(classInfo, null, null);
}

private DependentResourceAugmentedClassInfo(ClassInfo classInfo, AnnotationInstance dependentAnnotationFromController) {
private DependentResourceAugmentedClassInfo(ClassInfo classInfo, AnnotationInstance dependentAnnotationFromController,
String reconcilerName) {
super(classInfo, DEPENDENT_RESOURCE, 2,
Optional.ofNullable(dependentAnnotationFromController)
.map(a -> a.value("name"))
.map(AnnotationValue::asString)
.filter(Predicate.not(String::isBlank))
// note that this should match DependentResource.getDefaultNameFor implementation)
.orElse(classInfo.name().toString()));
.orElse(classInfo.name().toString()),
reconcilerName);
this.dependentAnnotationFromController = dependentAnnotationFromController;
}

public static DependentResourceAugmentedClassInfo createFor(ClassInfo classInfo,
AnnotationInstance dependentAnnotationFromController, IndexView index, Logger log,
Map<String, Object> context) {
final var info = new DependentResourceAugmentedClassInfo(classInfo, dependentAnnotationFromController);
Map<String, Object> context, String reconcilerName) {
final var info = new DependentResourceAugmentedClassInfo(classInfo, dependentAnnotationFromController, reconcilerName);
info.augmentIfKept(index, log, context);
return info;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
package io.quarkiverse.operatorsdk.common;

import static io.quarkiverse.operatorsdk.common.ClassLoadingUtils.loadClass;
import static io.quarkiverse.operatorsdk.common.Constants.CUSTOM_RESOURCE;
import static io.quarkiverse.operatorsdk.common.Constants.HAS_METADATA;
import static io.quarkiverse.operatorsdk.common.Constants.OBJECT;
import static io.quarkiverse.operatorsdk.common.Constants.*;

import java.util.Map;
import java.util.Optional;
Expand Down Expand Up @@ -65,14 +63,21 @@ public ReconciledResourceAugmentedClassInfo asResourceTargeting() {
}

@SuppressWarnings("rawtypes")
public static ReconciledAugmentedClassInfo createFor(ClassInfo resourceCI, String reconcilerName,
public static ReconciledAugmentedClassInfo createFor(ResourceAssociatedAugmentedClassInfo parent, ClassInfo resourceCI,
String reconcilerName,
IndexView index, Logger log, Map<String, Object> context) {
var isResource = false;
var isCR = false;
var isGenericKubernetesResource = false;
try {
isResource = ClassUtils.isImplementationOf(index, resourceCI, HAS_METADATA);
if (isResource) {
isCR = JandexUtil.isSubclassOf(index, resourceCI, CUSTOM_RESOURCE);
if (!isCR) {
// check if the target resource is a generic one
isGenericKubernetesResource = JandexUtil.isSubclassOf(index, parent.classInfo(),
GENERIC_KUBERNETES_DEPENDENT_RESOURCE);
}
}
} catch (BuildException e) {
log.errorv(
Expand All @@ -83,7 +88,8 @@ public static ReconciledAugmentedClassInfo createFor(ClassInfo resourceCI, Strin
ReconciledAugmentedClassInfo reconciledInfo;
if (isCR) {
reconciledInfo = new CustomResourceAugmentedClassInfo(resourceCI, reconcilerName);
} else if (isResource) {
} else if (isResource && !isGenericKubernetesResource) {
// only record detailed information if the target resource is not generic
reconciledInfo = new ReconciledResourceAugmentedClassInfo<>(resourceCI, HAS_METADATA, 0,
reconcilerName);
} else {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.quarkiverse.operatorsdk.common;

import java.util.Map;
import java.util.Objects;

import org.jboss.jandex.ClassInfo;
import org.jboss.jandex.DotName;
Expand All @@ -13,12 +14,18 @@ public class ReconciledResourceAugmentedClassInfo<T extends HasMetadata> extends

public static final String STATUS = "status";
protected boolean hasStatus;
private final String id;

protected ReconciledResourceAugmentedClassInfo(ClassInfo classInfo,
DotName extendedOrImplementedClass, int expectedParameterTypesCardinality,
String associatedReconcilerName) {
super(classInfo, extendedOrImplementedClass, expectedParameterTypesCardinality,
associatedReconcilerName);
id = fullResourceName() + version();
}

public String id() {
return id;
}

public String fullResourceName() {
Expand Down Expand Up @@ -55,4 +62,16 @@ protected boolean hasStatus(IndexView index) {
public boolean hasNonVoidStatus() {
return hasStatus;
}

@Override
public boolean equals(Object o) {
if (!(o instanceof ReconciledResourceAugmentedClassInfo<?> that))
return false;
return Objects.equals(id, that.id);
}

@Override
public int hashCode() {
return Objects.hashCode(id);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ protected void doAugment(IndexView index, Logger log, Map<String, Object> contex
dependentConfig.value("type"),
DependentResource.class, index);
final var dependent = DependentResourceAugmentedClassInfo.createFor(dependentType, dependentConfig, index,
log, context);
log, context, nameOrFailIfUnset());
final var dependentName = dependent.nameOrFailIfUnset();
final var dependentTypeName = dependentType.name().toString();
if (dependentResources.containsKey(dependentName)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,18 @@
public class ResourceAssociatedAugmentedClassInfo extends SelectiveAugmentedClassInfo {
private final String name;
private ReconciledAugmentedClassInfo<?> resourceInfo;
private final String reconcilerName;

protected ResourceAssociatedAugmentedClassInfo(ClassInfo classInfo,
DotName extendedOrImplementedClass, int expectedParameterTypesCardinality, String name) {
this(classInfo, extendedOrImplementedClass, expectedParameterTypesCardinality, name, null);
}

protected ResourceAssociatedAugmentedClassInfo(ClassInfo classInfo,
DotName extendedOrImplementedClass, int expectedParameterTypesCardinality, String name, String reconcilerName) {
super(classInfo, extendedOrImplementedClass, expectedParameterTypesCardinality);
this.name = name;
this.reconcilerName = reconcilerName != null ? reconcilerName : name;
}

public DotName resourceTypeName() {
Expand All @@ -42,7 +49,7 @@ protected void doAugment(IndexView index, Logger log, Map<String, Object> contex
+ "' has not been found in the Jandex index so it cannot be introspected. Please index your classes with Jandex.");
}

resourceInfo = ReconciledAugmentedClassInfo.createFor(primaryCI, name, index, log, context);
resourceInfo = ReconciledAugmentedClassInfo.createFor(this, primaryCI, reconcilerName, index, log, context);
}

public ReconciledAugmentedClassInfo<?> associatedResourceInfo() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ boolean shouldApply() {
* Generates the CRD in the location specified by the output target, using the specified CRD
* generation configuration only if generation has been requested by call
* {@link #scheduleForGenerationIfNeeded(CustomResourceAugmentedClassInfo, Map, Set)} or
* {@link #withCustomResource(Class, String, String)}
* {@link #withCustomResource(Class, String)}
*
* @param outputTarget the {@link OutputTargetBuildItem} specifying where the CRDs
* should be generated
Expand Down Expand Up @@ -101,7 +101,7 @@ CRDGenerationInfo generate(OutputTargetBuildItem outputTarget,
return new CRDGenerationInfo(shouldApply(), validateCustomResources, converted, generated);
}

private boolean needsGeneration(Map<String, CRDInfo> existingCRDInfos, Set<String> changedClassNames, String targetCRName) {
private boolean needsGeneration(Map<String, CRDInfo> existingCRDInfos, Set<String> changedClassNames) {
final boolean[] generateCurrent = { true }; // request CRD generation by default
crdConfiguration.versions().forEach(v -> {
final var crd = existingCRDInfos.get(v);
Expand All @@ -122,7 +122,7 @@ private boolean needsGeneration(Map<String, CRDInfo> existingCRDInfos, Set<Strin
// we've looked at all the changed classes and none have been changed for this CR/version: do not regenerate CRD
log.infov(
"''{0}'' CRD generation was skipped for ''{1}'' because no changes impacting the CRD were detected",
v, targetCRName);
v, crd.getCrdName());
generateCurrent[0] = false;
});
return generateCurrent[0];
Expand All @@ -131,21 +131,19 @@ private boolean needsGeneration(Map<String, CRDInfo> existingCRDInfos, Set<Strin
boolean scheduleForGenerationIfNeeded(CustomResourceAugmentedClassInfo crInfo,
Map<String, CRDInfo> existingCRDInfos, Set<String> changedClasses) {
var scheduleCurrent = true;
final String targetCRName = crInfo.asResourceTargeting().fullResourceName();

if (existingCRDInfos != null && !existingCRDInfos.isEmpty()) {
scheduleCurrent = needsGeneration(existingCRDInfos, changedClasses, targetCRName);
scheduleCurrent = needsGeneration(existingCRDInfos, changedClasses);
}

if (scheduleCurrent) {
withCustomResource(crInfo.loadAssociatedClass(), targetCRName, crInfo.getAssociatedReconcilerName().orElse(null));
withCustomResource(crInfo.loadAssociatedClass(), crInfo.getAssociatedReconcilerName().orElse(null));
}

return scheduleCurrent;
}

public void withCustomResource(Class<? extends CustomResource<?, ?>> crClass, String crdName,
String associatedControllerName) {
public void withCustomResource(Class<? extends CustomResource<?, ?>> crClass, String associatedControllerName) {
// first check if the CR is not filtered out
if (crdConfiguration.excludeResources().map(excluded -> excluded.contains(crClass.getName())).orElse(false)) {
log.infov("CRD generation was skipped for ''{0}'' because it was excluded from generation", crClass.getName());
Expand All @@ -159,7 +157,7 @@ public void withCustomResource(Class<? extends CustomResource<?, ?>> crClass, St
generator = new CRDGenerator().withParallelGenerationEnabled(crdConfiguration.generateInParallel());
}
final var info = CustomResourceInfo.fromClass(crClass);
crMappings.add(info, crdName, associatedControllerName);
crMappings.add(info, associatedControllerName);
generator.customResources(info);
needGeneration = true;
} catch (Exception e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,22 +52,22 @@ GeneratedCRDInfoBuildItem generateCRDs(
final ReconciledAugmentedClassInfo<?> associatedResource = raci.associatedResourceInfo();
if (associatedResource.isCR()) {
final var crInfo = associatedResource.asResourceTargeting();
final String targetCRName = crInfo.fullResourceName();
final String crId = crInfo.id();

// if the primary resource is unowned, mark it as "scheduled" (i.e. already handled) so that it doesn't get considered if all CRDs generation is requested
if (!operatorConfiguration.isControllerOwningPrimary(raci.nameOrFailIfUnset())) {
scheduledForGeneration.add(targetCRName);
scheduledForGeneration.add(crId);
} else {
// When we have a live reload, check if we need to regenerate the associated CRD
Map<String, CRDInfo> crdInfos = Collections.emptyMap();
if (liveReload.isLiveReload()) {
crdInfos = storedCRDInfos.getCRDInfosFor(targetCRName);
crdInfos = storedCRDInfos.getCRDInfosFor(crId);
}

// schedule the generation of associated primary resource CRD if required
if (crdGeneration.scheduleForGenerationIfNeeded((CustomResourceAugmentedClassInfo) crInfo, crdInfos,
changedClasses)) {
scheduledForGeneration.add(targetCRName);
scheduledForGeneration.add(crId);
}
}
}
Expand All @@ -80,9 +80,8 @@ GeneratedCRDInfoBuildItem generateCRDs(
Map.of(CustomResourceAugmentedClassInfo.EXISTING_CRDS_KEY, scheduledForGeneration))
.map(CustomResourceAugmentedClassInfo.class::cast)
.forEach(cr -> {
final var targetCRName = cr.fullResourceName();
crdGeneration.withCustomResource(cr.loadAssociatedClass(), targetCRName, null);
log.infov("Will generate CRD for non-reconciler bound resource: {0}", targetCRName);
crdGeneration.withCustomResource(cr.loadAssociatedClass(), null);
log.infov("Will generate CRD for non-reconciler bound resource: {0}", cr.fullResourceName());
});
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ public Map<String, ResourceInfo> getResourceInfos(String resourceFullName) {
return infos;
}

public void add(io.fabric8.crdv2.generator.CustomResourceInfo info, String crdName, String associatedControllerName) {
public void add(io.fabric8.crdv2.generator.CustomResourceInfo info, String associatedControllerName) {
final var version = info.version();
final var crdName = info.crdName();
final var versionsForCR = resourceFullNameToVersionToInfos.computeIfAbsent(crdName, s -> new HashMap<>());
final var cri = versionsForCR.get(version);
if (cri != null) {
Expand All @@ -37,17 +38,16 @@ public void add(io.fabric8.crdv2.generator.CustomResourceInfo info, String crdNa
throw new IllegalStateException(msg);
}

final var converted = augment(info, crdName, associatedControllerName);
final var converted = augment(info, associatedControllerName);
versionsForCR.put(version, converted);
}

private static ResourceInfo augment(io.fabric8.crdv2.generator.CustomResourceInfo info,
String crdName, String associatedControllerName) {
private static ResourceInfo augment(io.fabric8.crdv2.generator.CustomResourceInfo info, String associatedControllerName) {
return new ResourceInfo(
info.group(), info.version(), info.kind(), info.singular(), info.plural(), info.shortNames(),
info.storage(),
info.served(), info.scope(), info.crClassName(),
info.statusClassName().map(ClassUtils::isStatusNotVoid).orElse(false), crdName,
info.statusClassName().map(ClassUtils::isStatusNotVoid).orElse(false), info.crdName(),
associatedControllerName);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ public class AllCRDGenerationTest {
static final QuarkusProdModeTest config = new QuarkusProdModeTest()
.setApplicationName("test")
.withApplicationRoot(
(jar) -> jar.addClasses(SimpleReconciler.class, SimpleCR.class, SimpleSpec.class, SimpleStatus.class,
External.class, ExternalV1.class, SimpleReconcilerV2.class, SimpleCRV2.class))
(jar) -> jar.addClasses(SimpleCR.class, SimpleSpec.class, SimpleStatus.class,
External.class, ExternalV1.class, SimpleCRV2.class, SimpleReconcilerV2.class))
.overrideConfigKey("quarkus.operator-sdk.crd.generate-all", "true");

@ProdBuildResults
Expand Down