fix(container-detail): display the entire command instead of the first piece only #6604
Conversation
…t piece Signed-off-by: GLEF1X <[email protected]>
GLEF1X
requested review from
dgolovin and
lstocchi
and removed request for
a team
Signed-off-by: GLEF1X <[email protected]>
| @@ -42,7 +42,7 @@ export interface ContainerInfo { | |||
| Names: string[]; | |||
| Image: string; | |||
| ImageID: string; | |||
| Command?: string; | |||
| Command?: readonly string[]; | |||
There was a problem hiding this comment.
From my understanding, this interface is inspired from dockerode definiton
There was a problem hiding this comment.
AFAIK, you're totally right. However, I changed it to readonly string[] because listContainers dynamically selects among various providers, leading to the use of workarounds such as podmanContainer.Command?.length > 0 ? podmanContainer.Command[0]: undefined. In my opinion, ContainerInfo should use an array to represent a command even though dockerode returns a string because it enables us to not lose data(in this case other parts of the command).
There was a problem hiding this comment.
Yes I looked at the podman repository and they are indeed sending an array compared to dockerode definition.
I would be in favour to get something closer to what podman provide, the changes you are providing only touch the internal api, so it would not have much side effect, however the ContainerInfo is also provided to the extension-api.
However for the extension-api we have something different, with an interface called SimpleContainerInfo which is some extending of dockerode.
And I am wondering if we change/fix it at one place we would not have to propagate the changes also elsewhere... Waiting for @benoitf opinion on the matter
| @@ -1031,7 +1031,7 @@ describe('listContainers', () => { | |||
| expect(container.StartedAt).toBe('2023-08-10T13:37:44.000Z'); | |||
| expect(container.pod).toBeUndefined(); | |||
| expect(container.Id).toBe('31a4b282691420be2611817f203765402d8da7e13cd530f80a6ddd1bb4aa63b4'); | |||
| expect(container.Command).toBe(undefined); | |||
| expect(container.Command).toBe(null); | |||
There was a problem hiding this comment.
I am not sure to understand why null would be used here instead of undefined
There was a problem hiding this comment.
AFAIK, libpodApi returns null if the command is empty, whereas dockerode returns undefined
Co-authored-by: axel7083 <[email protected]> Signed-off-by: Hlib Haranin <[email protected]>
Signed-off-by: GLEF1X <[email protected]>
Signed-off-by: GLEF1X [email protected]
What does this PR do?
This pull request modifies the UI to display all arguments passed to a container's run command rather than showing just the first piece for improved clarity on the processes being run within the container.
Screenshot / video of UI
Before
After
What issues does this PR fix or reference?
#6414 - Show all of Command in Summary
How to test this PR?