From 041f01199600fbdb4abc66cc92b59ae3df08b7e0 Mon Sep 17 00:00:00 2001 From: Charlie Drage Date: Fri, 12 Jul 2024 15:49:52 -0400 Subject: [PATCH] bug: fix name display issues with compose files (#8025) ### What does this PR do? We should be using Names from Podman container inspect to safely determine the name. Relying on labels unfortunatly causes issues. After much investigation, it's determined that the difference between a normal compose file and one that uses `container_name` for services, is ONLY the `Names` array returned in podman inspect. Labels do not change. This PR adds the following: * Uses Names rather than docker.compose.service labels in order to safely and more accurately determine a compose services name. This works by determining what the project name is and taking it away from Names array. Accurately does this even if container_name is supplied in compose.yaml ### Screenshot / video of UI ### What issues does this PR fix or reference? Closes https://github.com/containers/podman-desktop/issues/5028 ### How to test this PR? - [X] Tests are covering the bug fix or the new feature 1. Test with a "normal" file and see that `-1` is added to compose names. ```yaml services: redis-leader: image: redis:latest redis-replica: image: redis:latest command: redis-server --replicaof redis-leader 6379 web: image: quay.io/kompose/web ``` 2. Test with a compose file that uses `container_name`, and see that the container name is propagated: ```yaml services: redis-leader: container_name: redis-leader image: redis:latest ports: - "6379" redis-replica: container_name: redis-replica image: redis:latest ports: - "6379" command: redis-server --replicaof redis-leader 6379 web: container_name: web image: quay.io/kompose/web ports: - "8080:8080" ``` Signed-off-by: Charlie Drage --- .../src/lib/container/container-utils.spec.ts | 31 +++++++++++++++++++ .../src/lib/container/container-utils.ts | 24 +++++++++----- 2 files changed, 47 insertions(+), 8 deletions(-) diff --git a/packages/renderer/src/lib/container/container-utils.spec.ts b/packages/renderer/src/lib/container/container-utils.spec.ts index 6cdae35b4de..dd4213fc745 100644 --- a/packages/renderer/src/lib/container/container-utils.spec.ts +++ b/packages/renderer/src/lib/container/container-utils.spec.ts @@ -277,6 +277,37 @@ test('check parsing of container info without names', async () => { expect(name).toBe(''); }); +test('check that if a container is part of compose, it will use the Names field for output WITHOUT the project name even if there is a name for service', async () => { + const containerInfo = { + Id: 'container1', + Image: 'registry.k8s.io/pause:3.7', + Labels: { + 'com.docker.compose.project': 'compose', + 'com.docker.compose.service': 'compose_container', + }, + Names: ['/compose-compose_container-1'], + State: 'RUNNING', + } as unknown as ContainerInfo; + const name = containerUtils.getName(containerInfo); + expect(name).toBe('compose_container-1'); +}); + +test('test that if a container is part of compose, and that container_name has been specified, that means that the Names[0] should be used without the project name', async () => { + const containerInfo = { + Id: 'container1', + Image: 'registry.k8s.io/pause:3.7', + Labels: { + 'com.docker.compose.project': 'compose', + 'com.docker.compose.service': 'container_name', + }, + // If container_name was specified in the compose file, this should be used (without the project name). + Names: ['/container_name'], + State: 'RUNNING', + } as unknown as ContainerInfo; + const name = containerUtils.getName(containerInfo); + expect(name).toBe('container_name'); +}); + test('check parsing of container info without labels', async () => { const context = new ContextUI(); const containerInfo = { diff --git a/packages/renderer/src/lib/container/container-utils.ts b/packages/renderer/src/lib/container/container-utils.ts index 8cdc8bc531e..3fa29597af7 100644 --- a/packages/renderer/src/lib/container/container-utils.ts +++ b/packages/renderer/src/lib/container/container-utils.ts @@ -32,17 +32,25 @@ import { ContainerGroupInfoTypeUI } from './ContainerInfoUI'; export class ContainerUtils { getName(containerInfo: ContainerInfo) { - // part of a compose ? - const composeService = containerInfo.Labels?.['com.docker.compose.service']; - if (composeService) { - const composeContainerNumber = containerInfo.Labels?.['com.docker.compose.container-number']; - if (composeContainerNumber) { - return `${composeService}-${composeContainerNumber}`; - } - } + // If the container has no name, return an empty string. if (containerInfo.Names.length === 0) { return ''; } + + // Safely determine if this is a compose project or not by checking the project label. + const composeProject = containerInfo.Labels?.['com.docker.compose.project']; + + /* + When deploying with compose, the container name will be -- under Names[0]. + This is added to the container name to make it unique. + HOWEVER, if you specify container_name in the compose file, the container name will be whatever is is set to and + will not have either the project or service number. + Thus the easier way to show the correct name is to get the containerInfo.Labels?.['com.docker.compose.project'] label + remove it from the Names[0] and return the result. + */ + if (composeProject) { + return containerInfo.Names[0].replace(/^\//, '').replace(`${composeProject}-`, ''); + } return containerInfo.Names[0].replace(/^\//, ''); }