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

Review CLI #364

Closed
goldmann opened this issue Jan 24, 2019 · 10 comments

Comments

Projects
2 participants
@goldmann
Copy link
Contributor

commented Jan 24, 2019

We need to think if current CLI is how it is should be done, or maybe it is better to have multiple parsers having parameters for specific targets (generate, build, etc).

We currently use argparse, maybe it would be worth to check other framework?

To review:

  • How Cekit is currently or should be used?
  • How to make sure what switch is for what action?

Useful to read:

@goldmann

This comment has been minimized.

Copy link
Contributor Author

commented Feb 21, 2019

Investigation

The biggest issue in designing a nice CLI for CEKit is that we chain commands; the build phase requires generate phase to finish first. When you build the image using cekit build, the generate phase is executed either case.

Even more, a specific build option can even influence how the generate phase is executed, as an example we have the OSBS build engine which influences how the generate phase generate files. These are different from what would be generated when executed for a Docker build engine.

One option would be to hide the generate phase and do not expose it at all to the CLI. This would result in a very clean CLI where only two commands would be available: build and test.

On the other hand it's very handy to have the generate phase exposed to to investigate the generated files without the need to build the image.

Proposal

Considering all above I think the best option would be to:

  1. Drop the generate command from CLI, leaving only build and test
  2. Add --dry-run switch to the build phase that would do what current generate phase does.
@goldmann

This comment has been minimized.

Copy link
Contributor Author

commented Feb 21, 2019

@jmtd @rnc @luck3y Any comments?

@goldmann

This comment has been minimized.

Copy link
Contributor Author

commented Feb 21, 2019

I've played with a stub and here is the help output and some examples. Feedback welcome.

Please note that I haven't investigated if the options are still useful or not, this is just a conversion of the current CLI.

Main help

$ python new_cli.py --help
Usage: new_cli.py [OPTIONS] COMMAND [ARGS]...

Options:
  --descriptor TEXT               Path to image descriptor file [default: image.yaml]
  -v, --verbose                   Enable verbose output
  --version                       Print version and exit
  --work-dir TEXT                 Location of the working directory [default: ~/.cekit]
  --config TEXT                   Path to configuration file [default: ~/.cekit/config]
  --redhat                        Set default options for Red Hat internal infrastructure
  --package-manager [yum|microdnf]
                                  Package manager to use [default: yum]
  --help                          Show this message and exit.

Commands:
  build  Build container image
  test   Execute container image tests

The build command

python new_cli.py build --help
Usage: new_cli.py build [OPTIONS] COMMAND [ARGS]...

  Build container image

Options:
  --dry-run              Do not execute the build, just generate required files
  --overrides TEXT       Inline overrides in JSON format
  --overrides-file TEXT  Path to overrides file in YAML format
  --target TEXT          Path to directory where files should be generated [default: target]
  --add-help             Include generated help files in the image [added by default]
  --help                 Show this message and exit.

Commands:
  docker  Build using Docker engine
  osbs    Build using OSBS engine

Docker builder

python new_cli.py build docker --help
Usage: new_cli.py build docker [OPTIONS]

  Build using Docker engine

Options:
  --pull       Always try to fetch latest base image
  --no-squash  Do not squash the image after build is done
  --tag TEXT   Tag the image after build, can be used multiple times
  --help       Show this message and exit.

OSBS builder

python new_cli.py build osbs --help  
Usage: new_cli.py build osbs [OPTIONS]

  Build using OSBS engine

Options:
  --release              Execute a release build
  --tech-preview         Execute a tech preview build
  --user TEXT            User used to kick the build as
  --nowait               Do not wait for the task to finish
  --stage                Use stage environment
  --target TEXT          Override the default target
  --commit-message TEXT  Custom dist-git commit message
  --help                 Show this message and exit.

test command

python new_cli.py test --help      
Usage: new_cli.py test [OPTIONS]

  Execute container image tests

Options:
  --steps-url TEXT  Behave steps library [default: https://github.com/cekit/behave-test-steps.git]
  --wip             Run test scenarios tagged with @wip only
  --name TEXT       Run test scenario with the specified name
  --image TEXT      Image to run the tests against  [required]
  --help            Show this message and exit.

Some examples

Execute local Docker build with verbose output

$ cekit -v build docker

Execute tests for image example/image:1.0

$ cekit test --image example/image:1.0

Execute release build in OSBS

$ cekit build osbs --release

Execute build in OSBS and specifying overrides

$ cekit build --overrides '{"from": "abc/def:1.0"}' osbs

Please note that the position of options is not up to the user. Options needs to be specified where the belong. For exmaple the --overrides option cannot be added as the last argument to the execution.

@goldmann

This comment has been minimized.

Copy link
Contributor Author

commented Feb 21, 2019

Oh and the most important example :)

Generate files for Docker build engine without executing the build

$ cekit build --dry-run docker

Generate files for OSBS build engine without executing the build

$ cekit build --dry-run osbs
@luck3y

This comment has been minimized.

Copy link

commented Feb 21, 2019

Looks good to me, I only use generate to test cekit is mostly working from time to time, and --dry-run seem to cover that.

One question, does cekit test also imply build? I can't see how it could, since you have a bunch of other build options to specify (osbs etc etc). But just wondering if it has a default?

@goldmann

This comment has been minimized.

Copy link
Contributor Author

commented Feb 21, 2019

Thanks for the comment. No, the test command does not imply build. It expects a built container image. This is a problem on it's own because this may or may not be what you want to have.

Potentially we could build the image if --image is not provided, but we will end up with the same issue we had with the generate command...

Maybe the solution would be to have two endpoints/binaries: cekit and cekit-test that would share some commands. But I think this could be an overkill and I'm not sure if it's required even since it could be sorted easily with cekit build docker --tag custom:1.0 && cekit test --image custom:1.0.

@luck3y

This comment has been minimized.

Copy link

commented Feb 21, 2019

I like the use of --image custom:1.0, that looks like it could add a lot of flexibility when building from various branches in CI and testing etc.

@goldmann

This comment has been minimized.

Copy link
Contributor Author

commented Feb 21, 2019

This is no different from what we have today: cekit build --tag custom:1.0 && cekit test --tag custom:1.0

@luck3y

This comment has been minimized.

Copy link

commented Feb 21, 2019

Well, there you are then, done. :)

I'd forgotten about those params!

@goldmann

This comment has been minimized.

Copy link
Contributor Author

commented Feb 22, 2019

I started playing with the CLI documentation a bit and here is a preview:

$ python cekit/new_cli.py build --help
Usage: new_cli.py build [OPTIONS] COMMAND [ARGS]...

  Build container image using specified builder

  Executes a container image build using the selected builder. See commands below for currently
  supported builders.

  OVERRIDES

      You can specify overrides to modify the container image build. You     can read more about
      overrides in the documentation: https://docs.cekit.io/en/latest/overrides.html.

      Overrides can be specified inline (--overrides) or as a path to file (--overrides-file).

      Overrides can be specified multiple times.

      Please note that order matters; overrides on the right hand side take precedence! For
      example:

          $ cekit build --overrides '{"from": "custom/image:1.0"}' --overrides '{"from":
          "custom/image:2.0"}'

      Will change the 'from' key in the descriptor to 'custom/image:2.0'.

Options:
  --dry-run              Do not execute the build, just generate required files
  --overrides JSON       Inline overrides in JSON format
  --overrides-file PATH  Path to overrides file in YAML format
  --target PATH          Path to directory where files should be generated [default: target]
  --help                 Show this message and exit.

Commands:
  buildah  Build using Buildah engine
  docker   Build using Docker engine
  osbs     Build using OSBS engine

I think it's nice to have something like this.

goldmann added a commit to goldmann/cekit that referenced this issue Feb 26, 2019

@goldmann goldmann referenced this issue Feb 26, 2019

Merged

CLI rewrite #412

goldmann added a commit to goldmann/cekit that referenced this issue Feb 27, 2019

goldmann added a commit to goldmann/cekit that referenced this issue Mar 5, 2019

@goldmann goldmann closed this in #412 Mar 7, 2019

goldmann added a commit that referenced this issue Mar 7, 2019

@goldmann goldmann added this to Done in Release 3.0 Mar 11, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.