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: allow keeping deleted CRDs in test with configuration #2687

Merged
merged 1 commit into from
Feb 17, 2025
Merged
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 @@ -8,11 +8,16 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.time.Duration;
import java.util.ArrayDeque;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like imports need to be optimized…

import java.util.ArrayList;
import java.util.Deque;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.stream.Stream;
Expand Down Expand Up @@ -43,6 +48,8 @@ public class LocallyRunOperatorExtension extends AbstractOperatorExtension {

private static final Logger LOGGER = LoggerFactory.getLogger(LocallyRunOperatorExtension.class);
private static final int CRD_DELETE_TIMEOUT = 1000;
private static final Set<AppliedCRD> appliedCRDs = new HashSet<>();
private static final boolean deleteCRDs = Boolean.parseBoolean(System.getProperty("testsuite.deleteCRDs", "true"));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't be these rather here as other configs?
Config

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think we want to configure this per Test, but rather for the whole testsuite. I think that per test it can be confusing.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, good point,
I'm working this this now:
#2688
And here the CRD's will have to be managed manually, but fur sure be at least one step where the CRD is updated. But that should be fine too, so yes, if we see that this canont cover such use cases when in particular tests user wants to manage use cases we can still add a property also to builder.


private final Operator operator;
private final List<ReconcilerSpec> reconcilers;
Expand All @@ -51,7 +58,6 @@ public class LocallyRunOperatorExtension extends AbstractOperatorExtension {
private final List<Class<? extends CustomResource>> additionalCustomResourceDefinitions;
private final Map<Reconciler, RegisteredController> registeredControllers;
private final Map<String, String> crdMappings;
private static final LinkedList<AppliedCRD> appliedCRDs = new LinkedList<>();

private LocallyRunOperatorExtension(
List<ReconcilerSpec> reconcilers,
Expand Down Expand Up @@ -297,8 +303,10 @@ protected void after(ExtensionContext context) {

var kubernetesClient = getKubernetesClient();

while (!appliedCRDs.isEmpty()) {
deleteCrd(appliedCRDs.poll(), kubernetesClient);
var iterator = appliedCRDs.iterator();
while (iterator.hasNext()) {
deleteCrd(iterator.next(), kubernetesClient);
iterator.remove();
}

kubernetesClient.close();
Expand All @@ -320,6 +328,10 @@ protected void after(ExtensionContext context) {
}

private void deleteCrd(AppliedCRD appliedCRD, KubernetesClient client) {
if (!deleteCRDs) {
LOGGER.debug("Skipping deleting CRD because of configuration: {}", appliedCRD);
return;
}
try {
LOGGER.debug("Deleting CRD: {}", appliedCRD.crdString);
final var crd = client.load(new ByteArrayInputStream(appliedCRD.crdString.getBytes()));
Expand Down