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 container repository add/remove content commands #422

Closed
gerrod3 opened this issue Nov 11, 2021 · 4 comments · Fixed by #496
Closed

Add container repository add/remove content commands #422

gerrod3 opened this issue Nov 11, 2021 · 4 comments · Fixed by #496
Assignees
Labels
feature request New feature request (template-set)
Milestone

Comments

@gerrod3
Copy link
Contributor

gerrod3 commented Nov 11, 2021

Summary

Container has special commands for adding/removing & copying content to container repositories. The content command group could be used, but it will need to use the polymorphic feature since these commands are only available for container container repositories.

Examples

pulp container repository content add --repository foo --name "tag_name"
pulp container repository content add --repository foo --digest "manifest_digest"
pulp container repository content remove --repository foo --href "content_href"
pulp container repository content modify --repository foo --add-content '["content_href1", "content_href2"]

pulp container repository copy -t all --source foo --dest bar copy everything
pulp container repository copy -t tags --source foo --source-version 2 --dest bar copy all tags from version 2 of foo
pulp container repository copy -t manifests --source foo --dest bar --digests '["digest1", "digest2"] copy manifests w/ digest1 & digest2

@gerrod3 gerrod3 added the feature request New feature request (template-set) label Nov 11, 2021
@gerrod3 gerrod3 self-assigned this Nov 12, 2021
@lubosmj
Copy link
Member

lubosmj commented Nov 15, 2021

Nice overview. How about doing a similar thing as I did for the OSTree plugin (pulp/pulp-cli-ostree@b2d2a09)? Instead of doing:

pulp container repository content add --repository foo --name "tag_name"

You could do:

pulp container repository tag add --repository foo --name "tag_name" --tagged_manifest "sha256:xyz"

Where you specify the content type directly and a user will be able to distinguish between specific content types just in time.

Furthermore, the uniqueness of tags is defined like this: https://github.com/pulp/pulp_container/blob/b0231b24c939b48c12e72d805e5f46c50d17c7e0/pulp_container/app/models.py#L200. Thus, it is necessary to specify a tagged manifest as well when performing such a command.

@gerrod3
Copy link
Contributor Author

gerrod3 commented Nov 15, 2021

Not a bad idea, but one issue is that we want to be consistent with command structures across plugins. Right now ansible, file and python all use the content subgroup commands, but they don't have many content types like container and rpm (which also doesn't have full support yet). I think this is something we should discuss at the CLI meeting to agree on a standard so users don't have to memorize different command syntaxes for different plugins.

(Edit) I do like your idea and wanted to add some extra thoughts I had thinking about it. Currently if an object has multiple types we use the --type option to specify which object we intend the command to use. (See container repository commands for examples) But we technically already use subcommand groups as a type specifier for differentiating between different plugin objects. So maybe specifying objects of different types within a plugin through subcommand groups is already a simple and consistent structure for commands. Right now the content subcommand group allows adding/removing/listing multiple contents through the --type option, but it could easily be broken out into multiple subcommand groups. e.g.

pulp container repository content -t "blob/manifest/tag" list/add/remove

would become

pulp container repository blob list/add/remove
pulp container repository manifest list//add/remove
pulp container repository tag list/add/remove

The content command factory could be updated to generate commands like this.

@lubosmj
Copy link
Member

lubosmj commented Nov 16, 2021

Right now, the --type option includes all content types by default (

content_contexts.update({"all": PulpContentContext})
). I think we need to get rid of the all entry if we want to proceed further with the --type option.

Also, I wanted to create a command like this:

pulp ostree repository content add ref --name "blabla" --checksum "123123"

But, it is not possible to create a new subcommand for the subcommand content (maybe the statement from https://click.palletsprojects.com/en/8.0.x/commands/ is relevant here: It is currently not possible for chain commands to be nested. This will be fixed in future versions of Click.). That is why I decided to go with:

pulp ostree repository ref add --name "blabla" --checksum "123123"

Furthermore, this command respects the generic pulp-cli interface since you can change the name of the command (

kwargs["name"] = "content"
). Yet, implementing such content commands was a bit cumbersome for pulp_ostree (https://github.com/pulp/pulp-cli-ostree/blob/b2d2a0946d3039d7575a24e3520041c7b77d1baa/pulpcore/cli/ostree/repository.py#L107-L132).

@gerrod3
Copy link
Contributor Author

gerrod3 commented Nov 16, 2021

But, it is not possible to create a new subcommand for the subcommand content (maybe the statement from https://click.palletsprojects.com/en/8.0.x/commands/ is relevant here: It is currently not possible for chain commands to be nested. This will be fixed in future versions of Click.). That is why I decided to go with:

It is possible, chaining commands are something different.

content_group = repository_content_command(...)
content_group.add_command(your_command)
repository.add_command(content_group)

Furthermore, this command respects the generic pulp-cli interface since you can change the name of the command (

kwargs["name"] = "content"

). Yet, implementing such content commands was a bit cumbersome for pulp_ostree (https://github.com/pulp/pulp-cli-ostree/blob/b2d2a0946d3039d7575a24e3520041c7b77d1baa/pulpcore/cli/ostree/repository.py#L107-L132).

Yes, that is cumbersome since it is not how I intended the command factory to be used. That's the problem with trying to create a generic factory, it doesn't cover every use case.

@mdellweg mdellweg linked a pull request Feb 2, 2022 that will close this issue
gerrod3 added a commit to gerrod3/pulp-cli that referenced this issue Feb 3, 2022
gerrod3 added a commit to gerrod3/pulp-cli that referenced this issue Mar 22, 2022
gerrod3 added a commit to gerrod3/pulp-cli that referenced this issue Mar 22, 2022
gerrod3 added a commit to gerrod3/pulp-cli that referenced this issue Mar 22, 2022
gerrod3 added a commit to gerrod3/pulp-cli that referenced this issue Mar 22, 2022
gerrod3 added a commit to gerrod3/pulp-cli that referenced this issue Mar 22, 2022
gerrod3 added a commit to gerrod3/pulp-cli that referenced this issue Mar 22, 2022
gerrod3 added a commit to gerrod3/pulp-cli that referenced this issue Mar 22, 2022
gerrod3 added a commit to gerrod3/pulp-cli that referenced this issue Mar 22, 2022
gerrod3 added a commit to gerrod3/pulp-cli that referenced this issue Mar 22, 2022
gerrod3 added a commit to gerrod3/pulp-cli that referenced this issue Mar 31, 2022
gerrod3 added a commit to gerrod3/pulp-cli that referenced this issue Mar 31, 2022
gerrod3 added a commit to gerrod3/pulp-cli that referenced this issue Apr 8, 2022
gerrod3 added a commit to gerrod3/pulp-cli that referenced this issue Apr 12, 2022
gerrod3 added a commit to gerrod3/pulp-cli that referenced this issue Apr 12, 2022
@gerrod3 gerrod3 changed the title Add container repository add/remove/copy content commands Add container repository add/remove content commands Apr 14, 2022
gerrod3 added a commit to gerrod3/pulp-cli that referenced this issue Apr 14, 2022
@mdellweg mdellweg added this to the 0.15.0 milestone Apr 15, 2022
@mdellweg mdellweg linked a pull request Apr 15, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature request (template-set)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants