Skip to content
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.

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

Add a client for Podman #222

Open
wants to merge 30 commits into
base: main
Choose a base branch
from
Open

Add a client for Podman #222

wants to merge 30 commits into from

Conversation

bwateratmsft
Copy link
Contributor

@bwateratmsft bwateratmsft commented Jan 24, 2024

Closes #221

I have tested the following end-to-end (Podman v3):

  • Identity
  • ClientIdentity
  • ImageNameDefaults
  • System Commands
  • VersionCommand
  • CheckInstallCommand
  • InfoCommand
  • GetEventStreamCommand
  • LoginCommand BUSTED for Podman v3
  • LogoutCommand BUSTED for Podman v3
  • Image Commands
  • BuildImageCommand
  • ListImagesCommand
  • RemoveImagesCommand
  • PruneImagesCommand
  • PullImageCommand
  • TagImageCommand
  • InspectImagesCommand
  • PushImageCommand
  • Container Commands
  • RunContainerCommand
  • ExecContainerCommand
  • ListContainersCommand
  • StartContainersCommand
  • RestartContainersCommand
  • StopContainersCommand
  • RemoveContainersCommand
  • PruneContainersCommand
  • LogsForContainerCommand
  • InspectContainersCommand
  • ContainersStatsCommand N/A
  • Volume Commands
  • CreateVolumeCommand
  • ListVolumesCommand
  • RemoveVolumesCommand
  • PruneVolumesCommand
  • InspectVolumesCommand
  • Network Commands
  • CreateNetworkCommand
  • ListNetworksCommand
  • RemoveNetworksCommand
  • PruneNetworksCommand
  • InspectNetworksCommand
  • Context commands N/A
  • ListContextsCommand N/A
  • RemoveContextsCommand N/A
  • UseContextCommand N/A
  • InspectContextsCommand N/A
  • Filesystem commands
  • ListFilesCommand
  • StatPathCommand
  • ReadFileCommand
  • WriteFileCommand

I have tested the following end-to-end (Podman v4 via Podman Desktop):

  • Identity
  • ClientIdentity
  • ImageNameDefaults
  • System Commands
  • VersionCommand
  • CheckInstallCommand
  • InfoCommand
  • GetEventStreamCommand
  • LoginCommand
  • LogoutCommand
  • Image Commands
  • BuildImageCommand
  • ListImagesCommand
  • RemoveImagesCommand
  • PruneImagesCommand
  • PullImageCommand
  • TagImageCommand
  • InspectImagesCommand
  • PushImageCommand
  • Container Commands
  • RunContainerCommand
  • ExecContainerCommand
  • ListContainersCommand
  • StartContainersCommand
  • RestartContainersCommand
  • StopContainersCommand
  • RemoveContainersCommand
  • PruneContainersCommand
  • LogsForContainerCommand BUSTED for Podman Desktop
  • InspectContainersCommand
  • ContainersStatsCommand N/A
  • Volume Commands
  • CreateVolumeCommand
  • ListVolumesCommand
  • RemoveVolumesCommand
  • PruneVolumesCommand
  • InspectVolumesCommand
  • Network Commands
  • CreateNetworkCommand
  • ListNetworksCommand
  • RemoveNetworksCommand
  • PruneNetworksCommand
  • InspectNetworksCommand
  • Context commands N/A
  • ListContextsCommand N/A
  • RemoveContextsCommand N/A
  • UseContextCommand N/A
  • InspectContextsCommand N/A
  • Filesystem commands
  • ListFilesCommand
  • StatPathCommand
  • ReadFileCommand BUSTED for Podman Desktop
  • WriteFileCommand Couldn't test

Known issues:

  • Login fails on v3 because we lead with https://
  • Logout fails on v3 because we lead with https://
  • Container logs fails on v4 with error
  • Read files fails on v4 with error
  • Write files was not tested on v4 due to read file errors

@bwateratmsft bwateratmsft requested a review from a team as a code owner January 24, 2024 16:07
@@ -93,7 +93,7 @@ export type ImageNameDefaults = {
};

export type CommonCommandOptions = {
shellProvider?: IShell;
shellProvider?: IShell; // TODO: this seems to be unused
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: Do

Env?: Array<string>,
ExposedPorts?: Record<string, unknown> | null;

// TODO: validate these remaining properties
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: Do

/**
* The default argument given to `--format`
*/
public readonly defaultFormatForJson: string = "{{ json . }}";
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: should we match the default of "{{json .}}" without the extra whitespace?


const client = new PodmanClient();
const runner = new WslShellCommandRunnerFactory({ strict: true });
const testDockerfileContext = '/mnt/d/vscode-docker-extensibility/packages/vscode-container-client/src/test/buildContext';
Copy link
Contributor

Choose a reason for hiding this comment

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

Should use relative paths + path.resolve to map these files.

Comment on lines 22 to 25
// const client = new PodmanClient();
// const runner = new ShellStreamCommandRunnerFactory({ strict: true });
// const testDockerfileContext = 'D:\\vscode-docker-extensibility\\packages\\vscode-container-client\\src\\test\\buildContext';
// const testDockerfile = 'D:\\vscode-docker-extensibility\\packages\\vscode-container-client\\src\\test\\buildContext\\Dockerfile';
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove commented out code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The tests are meant to be run locally, with some things commented in/out, so I'd prefer to keep this code. I agree though that the paths should be changed to be relative.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i.e., I want to be able to quickly switch between running the whole set of tests against Podman in WSL, vs. running them locally in Podman Desktop, by commenting out something and uncommenting something else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a client for Podman
2 participants