Skip to content

drop un-used configmap #1880

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

Merged
ryanjbaxter merged 3 commits into from
Feb 22, 2025
Merged

drop un-used configmap #1880

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0c93dfc
cleanup
wind57 Feb 21, 2025
17fd6b7
cleanup
wind57 Feb 22, 2025
fd63724
formatting
wind57 Feb 22, 2025
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
2 changes: 1 addition & 1 deletion .../src/main/java/org/springframework/cloud/kubernetes/integration/tests/commons/Images.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public final class Images {

private static final String ISTIO_PILOT_TAR = ISTIO_PILOT.replace('/', '-') + ":" + istioVersion();

private static final String KAFKA = "confluentinc/cp-kafka";
private static final String KAFKA = "confluentinc/confluent-local";
Copy link
Contributor Author

@wind57 wind57 Feb 22, 2025

Choose a reason for hiding this comment

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

confluent has a new image, that simplifies work that we have to do when when dealing with kafka


private static final String KAFKA_TAR = KAFKA.replace('/', '-') + kafkaVersion();

Expand Down
3 changes: 0 additions & 3 deletions ...va/org/springframework/cloud/kubernetes/integration/tests/commons/native_client/Util.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,14 +267,11 @@ public void kafka(String namespace, Phase phase) {
deployment.getSpec().getTemplate().getSpec().getContainers().get(0).setImage(imageWithVersion);

V1Service service = (V1Service) yaml("kafka/kafka-service.yaml");
V1ConfigMap configMap = (V1ConfigMap) yaml("kafka/kafka-configmap-startup-script.yaml");
Copy link
Contributor Author

@wind57 wind57 Feb 22, 2025

Choose a reason for hiding this comment

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

we do not need the configmap (and the script it had inside it) to properly run kafka broker

Copy link
Contributor

@ryanjbaxter ryanjbaxter Feb 22, 2025

Choose a reason for hiding this comment

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

The new image has this?

Copy link
Contributor Author

@wind57 wind57 Feb 22, 2025

Choose a reason for hiding this comment

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

sort of, its an image that has KRaft enabled, by default, so that we don't have to do the hack we did here


if (phase.equals(Phase.CREATE)) {
createAndWait(namespace, configMap, null);
createAndWait(namespace, "kafka", deployment, service, null, false);
}
else if (phase.equals(Phase.DELETE)) {
deleteAndWait(namespace, configMap, null);
deleteAndWait(namespace, deployment, service, null);
}
}
Expand Down
2 changes: 1 addition & 1 deletion spring-cloud-kubernetes-test-support/src/main/resources/current-images.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ busybox:1.36.1
istio/istioctl:1.21.6
istio/proxyv2:1.21.6
istio/pilot:1.21.6
confluentinc/cp-kafka:7.2.1
confluentinc/confluent-local:7.9.0
rabbitmq:3-management
wiremock/wiremock:3.4.2
12 changes: 0 additions & 12 deletions ...loud-kubernetes-test-support/src/main/resources/kafka/kafka-configmap-startup-script.yaml
View file
Open in desktop

This file was deleted.

18 changes: 4 additions & 14 deletions spring-cloud-kubernetes-test-support/src/main/resources/kafka/kafka-deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,16 @@ spec:
# and this will cause this problem: https://github.com/confluentinc/cp-docker-images/blob/master/debian/kafka/include/etc/confluent/docker/configure#L58-L62
# Another solution is to rename the service.
enableServiceLinks: false
volumes:
- name: kafka-start-config-script
Copy link
Contributor Author

@wind57 wind57 Feb 22, 2025

Choose a reason for hiding this comment

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

drop the volume and the script

configMap:
name: kafka-start-config-script
defaultMode: 0755
containers:
- name: kafka-start-config-script
volumeMounts:
- name: kafka-start-config-script
mountPath: /tmp
command:
- /tmp/kafka_start.sh
image: confluentinc/cp-kafka
- name: kafka-broker
image: confluentinc/confluent-local
ports:
- containerPort: 9092
env:
- name: KAFKA_BROKER_ID
value: "1"
- name: KAFKA_NODE_ID
value: "1"
- name: KAFKA_LISTENER_SECURITY_PROTOCOL_MAP
value: "CONTROLLER:PLAINTEXT,PLAINTEXT:PLAINTEXT"
- name: KAFKA_ADVERTISED_LISTENERS
Expand All @@ -47,8 +39,6 @@ spec:
value: "1"
- name: KAFKA_PROCESS_ROLES
value: "broker,controller"
- name: KAFKA_NODE_ID
value: "1"
- name: KAFKA_LISTENERS
value: "PLAINTEXT://:9092,CONTROLLER://:9093"
- name: KAFKA_INTER_BROKER_LISTENER_NAME
Expand Down
Loading