Skip to content

Commit 8b6464b

Browse files
scholzjscholzj
authored
Improve the UX of the v1 API Conversion tool (#12175)
Signed-off-by: Jakub Scholz <www@scholzj.com>
1 parent e59f6a2 commit 8b6464b

5 files changed

Lines changed: 110 additions & 20 deletions

File tree

‎v1-api-conversion/src/main/java/io/strimzi/kafka/api/conversion/v1/cli/AbstractCommand.java‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ public abstract class AbstractCommand implements Runnable {
2121

2222
protected static final ApiVersion FROM_API_VERSION = ApiVersion.V1BETA2;
2323
protected static final ApiVersion TO_API_VERSION = ApiVersion.V1;
24-
protected static final String STRIMZI_API = "kafka.strimzi.io";
2524
protected static final Set<String> STRIMZI_KINDS = Set.of(
2625
"Kafka",
2726
"KafkaConnect",
@@ -48,6 +47,21 @@ public abstract class AbstractCommand implements Runnable {
4847
"StrimziPodSet", "core.strimzi.io"
4948
);
5049

50+
// Mapping between kinds and the CRD names used to get the right CRD from the Kubernetes API
51+
@SuppressWarnings("SpellCheckingInspection")
52+
protected static final Map<String, String> CRD_NAMES = Map.of(
53+
"Kafka", "kafkas.kafka.strimzi.io",
54+
"KafkaConnect", "kafkaconnects.kafka.strimzi.io",
55+
"KafkaBridge", "kafkabridges.kafka.strimzi.io",
56+
"KafkaMirrorMaker2", "kafkamirrormaker2s.kafka.strimzi.io",
57+
"KafkaTopic", "kafkatopics.kafka.strimzi.io",
58+
"KafkaUser", "kafkausers.kafka.strimzi.io",
59+
"KafkaConnector", "kafkaconnectors.kafka.strimzi.io",
60+
"KafkaRebalance", "kafkarebalances.kafka.strimzi.io",
61+
"KafkaNodePool", "kafkanodepools.kafka.strimzi.io",
62+
"StrimziPodSet", "strimzipodsets.core.strimzi.io"
63+
);
64+
5165
@CommandLine.Spec
5266
CommandLine.Model.CommandSpec spec;
5367

‎v1-api-conversion/src/main/java/io/strimzi/kafka/api/conversion/v1/cli/ConvertResourceCommand.java‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import io.fabric8.kubernetes.api.model.GenericKubernetesResourceList;
99
import io.fabric8.kubernetes.client.KubernetesClient;
1010
import io.fabric8.kubernetes.client.KubernetesClientBuilder;
11+
import io.fabric8.kubernetes.client.KubernetesClientException;
1112
import io.fabric8.kubernetes.client.dsl.MixedOperation;
1213
import io.fabric8.kubernetes.client.dsl.Resource;
1314
import io.strimzi.kafka.api.conversion.v1.converter.ApiConversionFailedException;
@@ -74,8 +75,14 @@ protected List<GenericKubernetesResource> get(String kind, String namespace, boo
7475
* @return Updated custom resource
7576
*/
7677
protected GenericKubernetesResource replace(String kind, GenericKubernetesResource cr) {
77-
MixedOperation<GenericKubernetesResource, GenericKubernetesResourceList, Resource<GenericKubernetesResource>> op = Utils.versionedOperation(client, kind, STRIMZI_GROUPS.get(kind), TO_API_VERSION);
78-
return op.inNamespace(cr.getMetadata().getNamespace()).resource(cr).update();
78+
try {
79+
MixedOperation<GenericKubernetesResource, GenericKubernetesResourceList, Resource<GenericKubernetesResource>> op = Utils.versionedOperation(client, kind, STRIMZI_GROUPS.get(kind), TO_API_VERSION);
80+
GenericKubernetesResource replaced = op.inNamespace(cr.getMetadata().getNamespace()).resource(cr).update();
81+
println(cr.getKind() + " resource named " + cr.getMetadata().getName() + " in namespace " + cr.getMetadata().getNamespace() + " has been replaced in Kubernetes");
82+
return replaced;
83+
} catch (KubernetesClientException e) {
84+
throw new RuntimeException("Failed to updated the converted " + kind + " resource named " + cr.getMetadata().getName() + " in namespace " + cr.getMetadata().getNamespace() + ".", e);
85+
}
7986
}
8087

8188
/**
@@ -177,6 +184,10 @@ public void run() {
177184
boolean allNamespaces;
178185
client = new KubernetesClientBuilder().build();
179186

187+
// Pre-check for more user-friendliness
188+
println("Checking that the CRDs are present and have the desired API versions.");
189+
Utils.checkCrdsHaveApiVersions(client, CRD_NAMES.values(), FROM_API_VERSION, TO_API_VERSION);
190+
180191
// Handle the --namespace and --all-namespaces options
181192
if (exclusive == null) {
182193
namespace = client.getNamespace();

‎v1-api-conversion/src/main/java/io/strimzi/kafka/api/conversion/v1/cli/CrdUpgradeCommand.java‎

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -17,28 +17,12 @@
1717

1818
import java.util.ArrayList;
1919
import java.util.List;
20-
import java.util.Map;
2120

2221
/**
2322
* Upgrades the API versions in CRDs to store the new v1 version only
2423
*/
2524
@CommandLine.Command(name = "crd-upgrade", aliases = {"crd"}, description = "Upgrades the Strimzi CRDs and CRs to use v1beta2 version")
2625
public class CrdUpgradeCommand extends AbstractCommand {
27-
// Mapping between kinds and the CRD names used to get the right CRD from the Kubernetes API
28-
@SuppressWarnings("SpellCheckingInspection")
29-
final static Map<String, String> CRD_NAMES = Map.of(
30-
"Kafka", "kafkas.kafka.strimzi.io",
31-
"KafkaConnect", "kafkaconnects.kafka.strimzi.io",
32-
"KafkaBridge", "kafkabridges.kafka.strimzi.io",
33-
"KafkaMirrorMaker2", "kafkamirrormaker2s.kafka.strimzi.io",
34-
"KafkaTopic", "kafkatopics.kafka.strimzi.io",
35-
"KafkaUser", "kafkausers.kafka.strimzi.io",
36-
"KafkaConnector", "kafkaconnectors.kafka.strimzi.io",
37-
"KafkaRebalance", "kafkarebalances.kafka.strimzi.io",
38-
"KafkaNodePool", "kafkanodepools.kafka.strimzi.io",
39-
"StrimziPodSet", "strimzipodsets.core.strimzi.io"
40-
);
41-
4226
private KubernetesClient client;
4327

4428
/**
@@ -154,6 +138,10 @@ private void changeStoredVersionInStatus() {
154138
public void run() {
155139
client = new KubernetesClientBuilder().build();
156140

141+
// Pre-check for more user-friendliness
142+
println("Checking that the CRDs are present and have the desired API versions.");
143+
Utils.checkCrdsHaveApiVersions(client, CRD_NAMES.values(), FROM_API_VERSION, TO_API_VERSION);
144+
157145
// Change the stored version in CRD spec to v1beta2
158146
println("Changing stored version in all Strimzi CRDs to " + TO_API_VERSION + ":");
159147
changeStoredVersionInSpec();

‎v1-api-conversion/src/main/java/io/strimzi/kafka/api/conversion/v1/utils/Utils.java‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,15 @@
66

77
import io.fabric8.kubernetes.api.model.GenericKubernetesResource;
88
import io.fabric8.kubernetes.api.model.GenericKubernetesResourceList;
9+
import io.fabric8.kubernetes.api.model.apiextensions.v1.CustomResourceDefinition;
910
import io.fabric8.kubernetes.client.KubernetesClient;
1011
import io.fabric8.kubernetes.client.dsl.MixedOperation;
1112
import io.fabric8.kubernetes.client.dsl.Resource;
1213
import io.strimzi.api.annotations.ApiVersion;
1314

15+
import java.util.Arrays;
16+
import java.util.Collection;
17+
1418
/**
1519
* Various utility methods useful during the conversion
1620
*/
@@ -23,9 +27,33 @@ public class Utils {
2327
* @param group Group
2428
* @param apiVersion API version
2529
*
26-
* @return Client for working with given Kubernetes resource
30+
* @return Client for working with a given Kubernetes resource
2731
*/
2832
public static MixedOperation<GenericKubernetesResource, GenericKubernetesResourceList, Resource<GenericKubernetesResource>> versionedOperation(KubernetesClient client, String kind, String group, ApiVersion apiVersion) {
2933
return client.genericKubernetesResources(group + "/" + apiVersion, kind);
3034
}
35+
36+
/**
37+
* Checks if the CRDs are installed and have the correct API versions. This is used as a pre-check to avoid running
38+
* the conversion tool against older Strimzi versions missing the `v1` CRD API or missing the CRDs entirely.
39+
*
40+
* @param client Kubernetes client
41+
* @param crdNames Collection of the CRD names
42+
* @param apiVersions Array of required API versions
43+
*/
44+
public static void checkCrdsHaveApiVersions(KubernetesClient client, Collection<String> crdNames, ApiVersion... apiVersions) {
45+
for (String crdName : crdNames) {
46+
CustomResourceDefinition crd = client.apiextensions().v1().customResourceDefinitions().withName(crdName).get();
47+
48+
if (crd == null) {
49+
throw new RuntimeException("CRD " + crdName + " is missing");
50+
}
51+
52+
for (ApiVersion apiVersion : apiVersions) {
53+
if (crd.getSpec().getVersions().stream().noneMatch(v -> apiVersion.toString().equals(v.getName()))) {
54+
throw new RuntimeException("CRD " + crdName + " is missing at least one of the required API versions " + Arrays.toString(apiVersions));
55+
}
56+
}
57+
}
58+
}
3159
}

‎v1-api-conversion/src/test/java/io/strimzi/kafka/api/conversion/v1/utils/UtilsIT.java‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/*
2+
* Copyright Strimzi authors.
3+
* License: Apache License 2.0 (see the file LICENSE or http://apache.org/licenses/LICENSE-2.0.html).
4+
*/
5+
package io.strimzi.kafka.api.conversion.v1.utils;
6+
7+
import io.fabric8.kubernetes.client.ConfigBuilder;
8+
import io.fabric8.kubernetes.client.KubernetesClient;
9+
import io.fabric8.kubernetes.client.KubernetesClientBuilder;
10+
import io.strimzi.api.annotations.ApiVersion;
11+
import io.strimzi.kafka.api.conversion.v1.cli.ConversionTestUtils;
12+
import io.strimzi.test.CrdUtils;
13+
import org.junit.jupiter.api.AfterAll;
14+
import org.junit.jupiter.api.Assertions;
15+
import org.junit.jupiter.api.BeforeAll;
16+
import org.junit.jupiter.api.Test;
17+
18+
import java.util.List;
19+
20+
import static org.hamcrest.CoreMatchers.containsString;
21+
import static org.hamcrest.MatcherAssert.assertThat;
22+
23+
public class UtilsIT {
24+
private static KubernetesClient client;
25+
26+
@BeforeAll
27+
static void setupEnvironment() {
28+
client = new KubernetesClientBuilder().withConfig(new ConfigBuilder().build()).build();
29+
ConversionTestUtils.createOrUpdateCrd(client, CrdUtils.CRD_KAFKA_NAME, ConversionTestUtils.CRD_V1_KAFKA);
30+
}
31+
32+
@AfterAll
33+
static void teardownEnvironment() {
34+
CrdUtils.deleteCrd(client, CrdUtils.CRD_KAFKA_NAME);
35+
client.close();
36+
}
37+
38+
@Test
39+
public void testMissingVersion() {
40+
RuntimeException ex = Assertions.assertThrows(RuntimeException.class, () -> Utils.checkCrdsHaveApiVersions(client, List.of("kafkas.kafka.strimzi.io"), ApiVersion.V1BETA2, ApiVersion.V1));
41+
assertThat(ex.getMessage(), containsString("CRD kafkas.kafka.strimzi.io is missing at least one of the required API versions [v1beta2, v1]"));
42+
}
43+
44+
@Test
45+
public void testMissingCrd() {
46+
RuntimeException ex = Assertions.assertThrows(RuntimeException.class, () -> Utils.checkCrdsHaveApiVersions(client, List.of("kafkaconnects.kafka.strimzi.io"), ApiVersion.V1BETA2, ApiVersion.V1));
47+
assertThat(ex.getMessage(), containsString("CRD kafkaconnects.kafka.strimzi.io is missing"));
48+
}
49+
}

0 commit comments

Comments
 (0)