Skip to content

Commit e84b538

Browse files
LEDfanLEDfan
committed
Fix bug with namespaces in kubernetes-additional-manifests + add integraion tests
1 parent 6da7f42 commit e84b538

3 files changed

Lines changed: 245 additions & 21 deletions

File tree

‎src/main/java/eu/openanalytics/containerproxy/backend/kubernetes/KubernetesBackend.java‎

Lines changed: 42 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,6 @@
2121
package eu.openanalytics.containerproxy.backend.kubernetes;
2222

2323
import java.io.ByteArrayInputStream;
24-
import java.io.File;
25-
import java.io.FileWriter;
2624
import java.io.IOException;
2725
import java.io.OutputStream;
2826
import java.net.URI;
@@ -39,7 +37,6 @@
3937
import java.util.stream.Collectors;
4038

4139
import javax.inject.Inject;
42-
import javax.json.JsonValue;
4340

4441
import org.apache.commons.io.IOUtils;
4542

@@ -76,9 +73,9 @@
7673
import io.fabric8.kubernetes.client.ConfigBuilder;
7774
import io.fabric8.kubernetes.client.DefaultKubernetesClient;
7875
import io.fabric8.kubernetes.client.KubernetesClient;
79-
import io.fabric8.kubernetes.client.NamespacedKubernetesClient;
8076
import io.fabric8.kubernetes.client.dsl.LogWatch;
8177
import io.fabric8.kubernetes.client.internal.readiness.Readiness;
78+
import io.fabric8.kubernetes.client.utils.Serialization;
8279

8380
public class KubernetesBackend extends AbstractContainerBackend {
8481

@@ -139,16 +136,6 @@ protected Container startContainer(ContainerSpec spec, Proxy proxy) throws Excep
139136

140137
String kubeNamespace = getProperty(PROPERTY_NAMESPACE, DEFAULT_NAMESPACE);
141138
String apiVersion = getProperty(PROPERTY_API_VERSION, DEFAULT_API_VERSION);
142-
143-
// create additional manifests
144-
for (String manifest : proxy.getSpec().getKubernetesAdditionalManifests()) {
145-
List<HasMetadata> objects = kubeClient.load(new ByteArrayInputStream(manifest.getBytes())).get();
146-
for (HasMetadata object : objects) {
147-
if (kubeClient.resource(object).fromServer().get() == null) {
148-
kubeClient.resource(object).createOrReplace();
149-
}
150-
}
151-
}
152139

153140
String[] volumeStrings = Optional.ofNullable(spec.getVolumes()).orElse(new String[] {});
154141
List<Volume> volumes = new ArrayList<>();
@@ -252,6 +239,10 @@ protected Container startContainer(ContainerSpec spec, Proxy proxy) throws Excep
252239
Pod patchedPod = podPatcher.patchWithDebug(startupPod, proxy.getSpec().getKubernetesPodPatchAsJsonpatch());
253240
final String effectiveKubeNamespace = patchedPod.getMetadata().getNamespace(); // use the namespace of the patched Pod, in case the patch changes the namespace.
254241
container.getParameters().put(PARAM_NAMESPACE, effectiveKubeNamespace);
242+
243+
// create additional manifests -> use the effective (i.e. patched) namespace if no namespace is provided
244+
createAdditionalManifstes(proxy, effectiveKubeNamespace);
245+
255246
Pod startedPod = kubeClient.pods().inNamespace(effectiveKubeNamespace).create(patchedPod);
256247

257248
// Workaround: waitUntilReady appears to be buggy.
@@ -306,6 +297,41 @@ protected Container startContainer(ContainerSpec spec, Proxy proxy) throws Excep
306297
return container;
307298
}
308299

300+
301+
/**
302+
* Creates the extra manifests/resources defined in the ProxySpec.
303+
*
304+
* The resource will only be created if it does not already exist.
305+
*/
306+
private void createAdditionalManifstes(Proxy proxy, String namespace) {
307+
for (HasMetadata fullObject: getAdditionManifestsAsObjects(proxy, namespace)) {
308+
if (kubeClient.resource(fullObject).fromServer().get() == null) {
309+
kubeClient.resource(fullObject).createOrReplace();
310+
}
311+
}
312+
}
313+
314+
/**
315+
* Converts the additional manifests of the spec into HasMetadat objects.
316+
* When the resource has no namespace definition, the provided namespace
317+
* parameter will be used.
318+
*/
319+
private List<HasMetadata> getAdditionManifestsAsObjects(Proxy proxy, String namespace) {
320+
ArrayList<HasMetadata> result = new ArrayList<HasMetadata>();
321+
for (String manifest : proxy.getSpec().getKubernetesAdditionalManifests()) {
322+
HasMetadata object = Serialization.unmarshal(new ByteArrayInputStream(manifest.getBytes())); // used to determine whether the manifest has specified a namespace
323+
324+
HasMetadata fullObject = kubeClient.load(new ByteArrayInputStream(manifest.getBytes())).get().get(0);
325+
if (object.getMetadata().getNamespace() == null) {
326+
// the load method (in some cases) automatically sets a namepsace when no namespace is provided
327+
// therefore we overwrite this namespace with the namsepace of the pod.
328+
fullObject.getMetadata().setNamespace(namespace);
329+
}
330+
result.add(fullObject);
331+
}
332+
return result;
333+
}
334+
309335
private boolean isServiceReady(Service service) {
310336
if (service == null) {
311337
return false;
@@ -352,11 +378,8 @@ protected void doStopProxy(Proxy proxy) throws Exception {
352378
if (service != null) kubeClient.services().inNamespace(kubeNamespace).delete(service);
353379

354380
// delete additional manifests
355-
for (String manifest : proxy.getSpec().getKubernetesAdditionalManifests()) {
356-
List<HasMetadata> objects = kubeClient.load(new ByteArrayInputStream(manifest.getBytes())).get();
357-
for (HasMetadata object : objects) {
358-
kubeClient.resource(object).delete();
359-
}
381+
for (HasMetadata fullObject: getAdditionManifestsAsObjects(proxy, kubeNamespace)) {
382+
kubeClient.resource(fullObject).delete();
360383
}
361384
}
362385
}

‎src/test/java/eu/openanalytics/containerproxy/test/proxy/TestIntegrationOnKube.java‎

Lines changed: 169 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import static org.junit.Assert.assertNotNull;
2626
import static org.junit.Assert.assertTrue;
2727

28+
import java.io.ByteArrayInputStream;
2829
import java.net.URI;
2930
import java.util.List;
3031
import java.util.Map;
@@ -64,11 +65,15 @@
6465
import io.fabric8.kubernetes.api.model.ContainerStatus;
6566
import io.fabric8.kubernetes.api.model.EnvVar;
6667
import io.fabric8.kubernetes.api.model.NamespaceBuilder;
68+
import io.fabric8.kubernetes.api.model.PersistentVolumeClaim;
69+
import io.fabric8.kubernetes.api.model.PersistentVolumeClaimList;
6770
import io.fabric8.kubernetes.api.model.Pod;
6871
import io.fabric8.kubernetes.api.model.PodList;
6972
import io.fabric8.kubernetes.api.model.Quantity;
7073
import io.fabric8.kubernetes.api.model.ResourceRequirements;
74+
import io.fabric8.kubernetes.api.model.Secret;
7175
import io.fabric8.kubernetes.api.model.SecretBuilder;
76+
import io.fabric8.kubernetes.api.model.SecretList;
7277
import io.fabric8.kubernetes.api.model.Service;
7378
import io.fabric8.kubernetes.api.model.ServiceAccountBuilder;
7479
import io.fabric8.kubernetes.api.model.ServiceList;
@@ -474,7 +479,7 @@ public void launchProxyWithPodPatches() throws Exception {
474479
}
475480

476481
/**
477-
* Test whethet the merging of properties works properly
482+
* Test whether the merging of properties works properly
478483
*/
479484
@Test
480485
public void launchProxyWithPatchesWithMerging() throws Exception {
@@ -524,6 +529,169 @@ public void launchProxyWithPatchesWithMerging() throws Exception {
524529
assertEquals(0, proxyService.getProxies(null, true).size());
525530
}
526531

532+
/**
533+
* Tests the creation and deleting of additional manifests.
534+
* The first manifest contains a namespace definition.
535+
* The second manifest does not contain a namespace definition, but in the end should have the same namespace as the pod.
536+
*/
537+
@Test
538+
public void launchProxyWithAdditionalManifests() throws Exception {
539+
final String overridenNamespace = "it-b9fa0a24-overriden";
540+
try {
541+
System.out.println(client);
542+
client.namespaces().create(new NamespaceBuilder()
543+
.withNewMetadata()
544+
.withName(overridenNamespace)
545+
.endMetadata()
546+
.build());
547+
548+
String specId = environment.getProperty("proxy.specs[8].id");
549+
550+
ProxySpec baseSpec = proxyService.findProxySpec(s -> s.getId().equals(specId), true);
551+
ProxySpec spec = proxyService.resolveProxySpec(baseSpec, null, null);
552+
Proxy proxy = proxyService.startProxy(spec, true);
553+
String containerId = proxy.getContainers().get(0).getId();
554+
555+
PodList podList = client.pods().inNamespace(overridenNamespace).list();
556+
assertEquals(1, podList.getItems().size());
557+
Pod pod = podList.getItems().get(0);
558+
assertEquals("Running", pod.getStatus().getPhase());
559+
assertEquals(overridenNamespace, pod.getMetadata().getNamespace());
560+
assertEquals("sp-pod-" + containerId, pod.getMetadata().getName());
561+
assertEquals(1, pod.getStatus().getContainerStatuses().size());
562+
ContainerStatus container = pod.getStatus().getContainerStatuses().get(0);
563+
assertEquals(true, container.getReady());
564+
assertEquals("openanalytics/shinyproxy-demo:latest", container.getImage());
565+
566+
PersistentVolumeClaimList claimList = client.persistentVolumeClaims().inNamespace(overridenNamespace).list();
567+
assertEquals(1, claimList.getItems().size());
568+
PersistentVolumeClaim claim = claimList.getItems().get(0);
569+
assertEquals(overridenNamespace, claim.getMetadata().getNamespace());
570+
assertEquals("manifests-pvc", claim.getMetadata().getName());
571+
572+
// secret has no namespace defined -> should be created in the namespace defined by the pod patches
573+
SecretList sercetList = client.secrets().inNamespace(overridenNamespace).list();
574+
assertEquals(2, sercetList.getItems().size());
575+
for (Secret secret : sercetList.getItems()) {
576+
if (secret.getMetadata().getName().startsWith("default-token")) {
577+
continue;
578+
}
579+
assertEquals(overridenNamespace, secret.getMetadata().getNamespace());
580+
assertEquals("manifests-secret", secret.getMetadata().getName());
581+
}
582+
583+
proxyService.stopProxy(proxy, false, true);
584+
585+
// Give Kube the time to clean
586+
Thread.sleep(2000);
587+
588+
// all pods should be deleted
589+
podList = client.pods().inNamespace(session.getNamespace()).list();
590+
assertEquals(0, podList.getItems().size());
591+
// all additional manifests should be deleted
592+
assertEquals(1, client.secrets().inNamespace(overridenNamespace).list().getItems().size());
593+
assertTrue(client.secrets().inNamespace(overridenNamespace).list()
594+
.getItems().get(0).getMetadata().getName().startsWith("default-token"));
595+
assertEquals(0, client.persistentVolumeClaims().inNamespace(overridenNamespace).list().getItems().size());
596+
597+
assertEquals(0, proxyService.getProxies(null, true).size());
598+
} finally {
599+
// just to be sure both the namespace and service account are cleaned up
600+
client.namespaces().withName(overridenNamespace).delete();
601+
}
602+
}
603+
604+
/**
605+
* Tests the creation and deleting of additional manifests.
606+
* The first manifest contains a namespace definition.
607+
* The second manifest does not contain a namespace definition, but in the end should have the same namespace as the pod.
608+
*
609+
* This is exactly the same test as the previous one, except that the PVC already exists (and should not be re-created).
610+
*/
611+
@Test
612+
public void launchProxyWithAdditionalManifestsOfWhichOneAlreadyExists() throws Exception {
613+
final String overridenNamespace = "it-b9fa0a24-overriden";
614+
try {
615+
System.out.println(client);
616+
client.namespaces().create(new NamespaceBuilder()
617+
.withNewMetadata()
618+
.withName(overridenNamespace)
619+
.endMetadata()
620+
.build());
621+
622+
// create the PVC
623+
String pvcSpec =
624+
"apiVersion: v1\n" +
625+
"kind: PersistentVolumeClaim\n" +
626+
"metadata:\n" +
627+
" name: manifests-pvc\n" +
628+
" namespace: it-b9fa0a24-overriden\n" +
629+
"spec:\n" +
630+
" storageClassName: standard\n" +
631+
" accessModes:\n" +
632+
" - ReadWriteOnce\n" +
633+
" resources:\n" +
634+
" requests:\n" +
635+
" storage: 5Gi";
636+
637+
client.load(new ByteArrayInputStream(pvcSpec.getBytes())).createOrReplace();
638+
639+
String specId = environment.getProperty("proxy.specs[8].id");
640+
641+
ProxySpec baseSpec = proxyService.findProxySpec(s -> s.getId().equals(specId), true);
642+
ProxySpec spec = proxyService.resolveProxySpec(baseSpec, null, null);
643+
Proxy proxy = proxyService.startProxy(spec, true);
644+
String containerId = proxy.getContainers().get(0).getId();
645+
646+
PodList podList = client.pods().inNamespace(overridenNamespace).list();
647+
assertEquals(1, podList.getItems().size());
648+
Pod pod = podList.getItems().get(0);
649+
assertEquals("Running", pod.getStatus().getPhase());
650+
assertEquals(overridenNamespace, pod.getMetadata().getNamespace());
651+
assertEquals("sp-pod-" + containerId, pod.getMetadata().getName());
652+
assertEquals(1, pod.getStatus().getContainerStatuses().size());
653+
ContainerStatus container = pod.getStatus().getContainerStatuses().get(0);
654+
assertEquals(true, container.getReady());
655+
assertEquals("openanalytics/shinyproxy-demo:latest", container.getImage());
656+
657+
PersistentVolumeClaimList claimList = client.persistentVolumeClaims().inNamespace(overridenNamespace).list();
658+
assertEquals(1, claimList.getItems().size());
659+
PersistentVolumeClaim claim = claimList.getItems().get(0);
660+
assertEquals(overridenNamespace, claim.getMetadata().getNamespace());
661+
assertEquals("manifests-pvc", claim.getMetadata().getName());
662+
663+
// secret has no namespace defined -> should be created in the namespace defined by the pod patches
664+
SecretList sercetList = client.secrets().inNamespace(overridenNamespace).list();
665+
assertEquals(2, sercetList.getItems().size());
666+
for (Secret secret : sercetList.getItems()) {
667+
if (secret.getMetadata().getName().startsWith("default-token")) {
668+
continue;
669+
}
670+
assertEquals(overridenNamespace, secret.getMetadata().getNamespace());
671+
assertEquals("manifests-secret", secret.getMetadata().getName());
672+
}
673+
674+
proxyService.stopProxy(proxy, false, true);
675+
676+
// Give Kube the time to clean
677+
Thread.sleep(2000);
678+
679+
// all pods should be deleted
680+
podList = client.pods().inNamespace(session.getNamespace()).list();
681+
assertEquals(0, podList.getItems().size());
682+
// all additional manifests should be deleted
683+
assertEquals(1, client.secrets().inNamespace(overridenNamespace).list().getItems().size());
684+
assertTrue(client.secrets().inNamespace(overridenNamespace).list()
685+
.getItems().get(0).getMetadata().getName().startsWith("default-token"));
686+
assertEquals(0, client.persistentVolumeClaims().inNamespace(overridenNamespace).list().getItems().size());
687+
688+
assertEquals(0, proxyService.getProxies(null, true).size());
689+
} finally {
690+
// just to be sure both the namespace and service account are cleaned up
691+
client.namespaces().withName(overridenNamespace).delete();
692+
}
693+
}
694+
527695
public static class TestConfiguration {
528696
@Bean
529697
@Primary

‎src/test/resources/application-test.yml‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,9 +108,42 @@ proxy:
108108
value:
109109
name: ADDED_VAR
110110
value: VALUE
111-
- id: 02_hello
111+
- id: 01_hello_manifests
112112
container-specs:
113113
- image: "openanalytics/shinyproxy-demo"
114114
cmd: ["R", "-e", "shinyproxy::run_01_hello()"]
115115
port-mapping:
116116
default: 3838
117+
kubernetes-pod-patches: |
118+
- op: replace
119+
path: /metadata/namespace
120+
value: it-b9fa0a24-overriden
121+
kubernetes-additional-manifests:
122+
- |
123+
apiVersion: v1
124+
kind: PersistentVolumeClaim
125+
metadata:
126+
name: manifests-pvc
127+
namespace: it-b9fa0a24-overriden
128+
spec:
129+
storageClassName: standard
130+
accessModes:
131+
- ReadWriteOnce
132+
resources:
133+
requests:
134+
storage: 5Gi
135+
- |
136+
apiVersion: v1
137+
kind: Secret
138+
metadata:
139+
name: manifests-secret
140+
type: Opaque
141+
data:
142+
password: cGFzc3dvcmQ=
143+
144+
- id: 02_hello
145+
container-specs:
146+
- image: "openanalytics/shinyproxy-demo"
147+
cmd: ["R", "-e", "shinyproxy::run_01_hello()"]
148+
port-mapping:
149+
default: 3838

0 commit comments

Comments
 (0)