feat: show warnings on pod creation and two containers use the same p…

…ort (#2240) (#2671) * feat: show warnings on pod creation and two containers use the same port (#2240) Signed-off-by: lstocchi <lstocchi@redhat.com> * update warning banner design and message Signed-off-by: lstocchi <lstocchi@redhat.com> * stop looping on item with a port Signed-off-by: lstocchi <lstocchi@redhat.com> * add test Signed-off-by: lstocchi <lstocchi@redhat.com> --------- Signed-off-by: lstocchi <lstocchi@redhat.com>
podman-desktop · Jun 1, 2023 · 235aec1 · 235aec1
1 parent bc5f44a
commit 235aec1
Show file tree

Hide file tree

Showing 5 changed files with 130 additions and 8 deletions.
diff --git a/packages/renderer/src/lib/container/ContainerInfoUI.ts b/packages/renderer/src/lib/container/ContainerInfoUI.ts
@@ -16,6 +16,8 @@
  * SPDX-License-Identifier: Apache-2.0
  ***********************************************************************/
 
+import type { Port } from '@podman-desktop/api';
+
 // type of groups
 export enum ContainerGroupInfoTypeUI {
   STANDALONE = 'standalone',
@@ -51,7 +53,7 @@ export interface ContainerInfoUI {
   state: string;
   uptime: string;
   startedAt: string;
-  ports: number[];
+  ports: Port[];
   portsAsString: string;
   displayPort: string;
   command: string;

diff --git a/packages/renderer/src/lib/container/container-utils.ts b/packages/renderer/src/lib/container/container-utils.ts
@@ -22,6 +22,7 @@ import { ContainerGroupInfoTypeUI } from './ContainerInfoUI';
 import moment from 'moment';
 import humanizeDuration from 'humanize-duration';
 import { filesize } from 'filesize';
+import type { Port } from '@podman-desktop/api';
 export class ContainerUtils {
   getName(containerInfo: ContainerInfo) {
     // part of a compose ?
@@ -77,12 +78,12 @@ export class ContainerUtils {
     return image;
   }
 
-  getPorts(containerInfo: ContainerInfo): number[] {
-    return containerInfo.Ports?.filter(port => port.PublicPort).map(port => port.PublicPort) || [];
+  getPorts(containerInfo: ContainerInfo): Port[] {
+    return containerInfo.Ports?.filter(port => port.PublicPort).map(port => port) || [];
   }
 
   getDisplayPort(containerInfo: ContainerInfo): string {
-    const ports = this.getPorts(containerInfo);
+    const ports = this.getPorts(containerInfo).map(port => port.PublicPort);
     if (ports.length === 0) {
       return '';
     }
@@ -208,7 +209,7 @@ export class ContainerUtils {
   }
 
   getPortsAsString(containerInfo: ContainerInfo): string {
-    const ports = this.getPorts(containerInfo);
+    const ports = this.getPorts(containerInfo).map(port => port.PublicPort);
     if (ports.length > 1) {
       return `${ports.join(', ')}`;
     } else if (ports.length === 1) {

diff --git a/packages/renderer/src/lib/pod/PodCreateFromContainers.spec.ts b/packages/renderer/src/lib/pod/PodCreateFromContainers.spec.ts
@@ -62,7 +62,45 @@ const podCreation: PodCreation = {
       engineId: 'podman',
       id: 'id',
       name: 'cont_1',
-      ports: [9090, 8080],
+      ports: [
+        {
+          PublicPort: 9090,
+          IP: 'ip',
+          PrivatePort: 80,
+          Type: 'type',
+        },
+        {
+          PublicPort: 8080,
+          IP: 'ip',
+          PrivatePort: 81,
+          Type: 'type',
+        },
+      ],
+    },
+  ],
+  name: 'pod',
+};
+
+const podCreationSamePortContainers: PodCreation = {
+  containers: [
+    {
+      engineId: 'podman',
+      id: 'id',
+      name: 'cont_1',
+      ports: [
+        {
+          PublicPort: 9090,
+          IP: 'ip',
+          PrivatePort: 80,
+          Type: 'type',
+        },
+        {
+          PublicPort: 8080,
+          IP: 'ip',
+          PrivatePort: 80,
+          Type: 'type',
+        },
+      ],
     },
   ],
   name: 'pod',
@@ -205,3 +243,21 @@ test('Show error if pod creation fails', async () => {
   const exposedPortsLabel = await screen.findByText('error create pod');
   expect(exposedPortsLabel).toBeInTheDocument();
 });
+
+test('Show warning if multiple containers use the same port', async () => {
+  providerInfos.set([providerInfo]);
+  podCreationHolder.set(podCreationSamePortContainers);
+
+  render(PodCreateFromContainers, {});
+  const warningLabel = await screen.findByLabelText('warning');
+  expect(warningLabel).toBeInTheDocument();
+});
+
+test('Do not show warning if multiple containers use different ports', async () => {
+  providerInfos.set([providerInfo]);
+  podCreationHolder.set(podCreation);
+
+  render(PodCreateFromContainers, {});
+  const warningLabel = screen.queryByLabelText('warning');
+  expect(warningLabel).not.toBeInTheDocument();
+});
diff --git a/packages/renderer/src/lib/pod/PodCreateFromContainers.svelte b/packages/renderer/src/lib/pod/PodCreateFromContainers.svelte
@@ -9,11 +9,15 @@ import type { Unsubscriber } from 'svelte/store';
 import ErrorMessage from '../ui/ErrorMessage.svelte';
 import StatusIcon from '../images/StatusIcon.svelte';
 import ContainerIcon from '../images/ContainerIcon.svelte';
+import { faTriangleExclamation } from '@fortawesome/free-solid-svg-icons';
+import Fa from 'svelte-fa/src/fa.svelte';
 
 let podCreation: PodCreation;
 let createInProgress = false;
 let createError = undefined;
 $: mapPortExposed = new Map<number, { exposed: boolean; container: string }>();
+let containersPorts: { containers: string[]; ports: number[] }[];
+$: containersPorts = [];
 
 let providers: ProviderInfo[] = [];
 $: providerConnections = providers
@@ -116,17 +120,48 @@ onMount(() => {
 
   podCreationUnsubscribe = podCreationHolder.subscribe(value => {
     podCreation = value;
+    const mapPortPrivate = new Map<number, string[]>();
     podCreation.containers.forEach(container => {
       container.ports.forEach(port => {
-        mapPortExposed.set(port, {
+        mapPortExposed.set(port.PublicPort, {
           exposed: true,
           container: container.name,
         });
+        mapPortPrivate.set(port.PrivatePort, [...(mapPortPrivate.get(port.PrivatePort) || []), container.name]);
       });
     });
+    mapPortPrivate.forEach((value, key) => {
+      if (value.length < 2) {
+        mapPortPrivate.delete(key);
+        return;
+      }
+      const indexContainersItem = getIndexSameContainersItems(value);
+      if (indexContainersItem !== undefined) {
+        containersPorts[indexContainersItem].ports.push(key);
+      } else {
+        containersPorts.push({
+          containers: value,
+          ports: [key],
+        });
+      }
+    });
   });
 });
 
+function getIndexSameContainersItems(containers: string[]): number | undefined {
+  let index = 0;
+  for (const entry of containersPorts) {
+    const isDiff =
+      containers.filter(arr1Item => !entry.containers.includes(arr1Item)).length > 0 ||
+      entry.containers.filter(arr1Item => !containers.includes(arr1Item)).length > 0;
+    if (!isDiff) {
+      return index;
+    }
+    index++;
+  }
+  return undefined;
+}
+
 onDestroy(() => {
   if (providersUnsubscribe) {
     providersUnsubscribe();
@@ -155,6 +190,33 @@ function updatePortExposure(port: number, checked: boolean) {
     <div class="m-5 p-6 h-full bg-charcoal-800 rounded-sm text-gray-700">
       <div class="w-4/5 min-w-[500px]">
         {#if podCreation}
+          {#if containersPorts.length > 0}
+            <div class="bg-charcoal-600 border-t-2 border-amber-500 p-4 mb-2" role="alert" aria-label="warning">
+              <div class="flex flex-row">
+                <div class="mr-3">
+                  <Fa size="18" class="text-amber-400" icon="{faTriangleExclamation}" />
+                </div>
+                <div class="flex flex-col">
+                  <div class="text-sm text-amber-400">Possible runtime error</div>
+                  {#each containersPorts as { containers, ports }}
+                    <div class="mt-1 text-sm text-white">
+                      Containers
+                      {#each containers as container, index}
+                        <span class="font-bold">{container}</span>
+                        {#if index === containers.length - 2}
+                          and
+                        {:else if index < containers.length - 1}
+                          ,
+                        {/if}
+                        {' '}
+                      {/each}
+                      use same <span class="font-bold">{ports.length > 1 ? 'ports' : 'port'} {ports.join(', ')}</span>.
+                    </div>
+                  {/each}
+                </div>
+              </div>
+            </div>
+          {/if}
           <div class="mb-2">
             <span class="block text-sm font-semibold rounded text-gray-400 dark:text-gray-400">Name of the pod:</span>
           </div>

diff --git a/packages/renderer/src/stores/creation-from-containers-store.ts b/packages/renderer/src/stores/creation-from-containers-store.ts
@@ -16,14 +16,15 @@
  * SPDX-License-Identifier: Apache-2.0
  ***********************************************************************/
 
+import type { Port } from '@podman-desktop/api';
 import type { Writable } from 'svelte/store';
 import { writable } from 'svelte/store';
 
 export interface PodCreationContainer {
   id: string;
   name: string;
   engineId: string;
-  ports: number[];
+  ports: Port[];
 }
 
 export interface PodCreation {