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.

Sign up for GitHub

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

Improve single-instance example #5829

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Improve single-instance example #5829

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/Installing.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ From there, you can trigger a new job or clone an existing one, e.g.:

[source,sh]
----
wget https://raw.githubusercontent.com/os-autoinst/os-autoinst-distri-example/main/scenario-definitions.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

Alternatively we could encourage using the async=1 flag. Then openQA can download the file itself on the fly.

openqa-cli schedule --monitor \
    async=1 SCENARIO_DEFINITIONS_YAML_FILE=https://raw.githubusercontent.com/os-autoinst/os-autoinst-distri-example/main/scenario-definitions.yaml \
    DISTRI=example VERSION=0 FLAVOR=DVD ARCH=x86_64 \
    TEST=simple_boot _GROUP_ID=0 BUILD=test \
    CASEDIR=https://github.com/os-autoinst/os-autoinst-distri-example.git \
    NEEDLES_DIR=%%CASEDIR%%/needles

Copy link
Member

Choose a reason for hiding this comment

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

Are there any disadvantages to it?

Copy link

Choose a reason for hiding this comment

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

Usage: openqa-cli schedule [OPTIONS] DISTRI=… VERSION=… FLAVOR=… ARCH=… [ISO=… …]

Options:
      --apibase <path>           API base, defaults to /api/v1
      --apikey <key>             API key
      --apisecret <secret>       API secret
      --host <host>              Target host, defaults to http://localhost
  -h, --help                     Show this summary of available options
      --osd                      Set target host to http://openqa.suse.de
      --o3                       Set target host to https://openqa.opensuse.org
      --param-file <param=file>  Load content of params from files instead of
                                 from command line arguments. Multiple params
                                 may be specified by adding the option
                                 multiple times
  -m, --monitor                  Wait until all jobs are done/cancelled and return
                                 non-zero exit code if at least on job has not
                                 passed/softfailed
      --name <name>              Name of this client, used by openQA to
                                 identify different clients via User-Agent
                                 header, defaults to "openqa-cli"
  -i, --poll-interval <seconds>  Specifies the poll interval used with --monitor
  -p, --pretty                   Pretty print JSON content
  -q, --quiet                    Do not print error messages to STDERR
  -v, --verbose                  Print HTTP response headers

async=1 is not mentioned in the help and it's not immediately obvious what it does (from a user perspective).

Does it just download the file or does it do other things as well? From the code snippet it looks like it is hard-coded to always download this specific file, i.e. if I specify a different URL it will not download the file I expect?

Copy link
Contributor

Choose a reason for hiding this comment

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

The CLI is just a client. Its help doesn't contain information about the server-side parameters. This particular flag is documented in the openQA documentation if I remember correctly.

Not sure what makes you think any paths are hardcore. Of course it'll download the file you specify.

Are there any disadvantages to it?

With the suggested change the example doesn't show anymore how to specify a local path which can also be very useful to test local modifications. Maybe we still want to have such an example somewhere in the documentation.

Copy link

Choose a reason for hiding this comment

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

I missed the part where the change is in the docs, not the codebase, my bad 😅

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 still don't find any clear explanation here of what async=1 does in this case. It does download it on the server side, right?

Copy link
Member

Choose a reason for hiding this comment

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

Iso.pm enqueues a minion job if async is set

Copy link

Choose a reason for hiding this comment

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

Is this the only thing that async=1 does or does it affect other things as well? i.e. if it just downloads the specified file, why is it called async? And if it has other behavior, are there any implications?

Copy link
Contributor

Choose a reason for hiding this comment

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

@perlpunk So the note at the bottom of https://open.qa/docs/#_spawning_multiple_jobs_based_on_templates_isos_post is not good enough? It explains what async=1 is intended to be used for.

And yes, it does not mention the SCENARIO_DEFINITIONS_YAML_FILE. Maybe we should add a note where we introduce the SCENARIO_DEFINITIONS_YAML_FILE parameter that one can specify an HTTP-URL with the async=1 parameter and that the download is then done server-side.

Copy link
Member

Choose a reason for hiding this comment

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

#5855

openqa-cli schedule --monitor \
--param-file SCENARIO_DEFINITIONS_YAML=scenario-definitions.yaml \
Comment on lines +70 to 72
Copy link
Member

Choose a reason for hiding this comment

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

According to http://open.qa/docs/#_create_and_monitor_openqa_jobs_from_within_the_ci_runner
one should be able to do

SCENARIO_DEFINITIONS_YAML_FILE

Suggested change
wget https://raw.githubusercontent.com/os-autoinst/os-autoinst-distri-example/main/scenario-definitions.yaml
openqa-cli schedule --monitor \
--param-file SCENARIO_DEFINITIONS_YAML=scenario-definitions.yaml \
openqa-cli schedule --monitor \
--param-file SCENARIO_DEFINITIONS_YAML_FILE=https://raw.githubusercontent.com/os-autoinst/os-autoinst-distri-example/main/scenario-definitions.yaml \

then but something feels off here. Why would we have a parameter SCENARIO_DEFINITIONS_YAML and one SCENARIO_DEFINITIONS_YAML_FILE which can take an URL but for that we normally use the suffix _URL?

I suggest to check on that before continuing here. Just downloading a file locally to pass it unaltered to a remote server which then reads the git repo anyway sounds wrong and is not what we should encourage users to do. Maybe that's even a missing feature in openQA which I expected to be present already.

Copy link
Contributor

@Martchus Martchus Aug 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We have SCENARIO_DEFINITIONS_YAML_FILE because initially it was only able to take a local file. I added the ability to specify a URL later for the webhook-based GitHub integration. And it only works if async=1 is used (which is a limitation that makes sense).

Note that SCENARIO_DEFINITIONS_YAML takes the YAML contents itself. It only works with a file here due to --param-file which is a client-side feature - which means the file needs to be downloaded on the client like in @perlpunk's wget example. (When using SCENARIO_DEFINITIONS_YAML_FILE the path would be located by the server.)

I'd probably prefer #5829 (comment). Note that your example doesn't work at all. It tells the client to lookup the local file https://raw.githubusercontent.com/os-autoinst/os-autoinst-distri-example/main/scenario-definitions.yaml and upload its contents via SCENARIO_DEFINITIONS_YAML_FILE. Also note the use of async=1 in my example.

DISTRI=example VERSION=0 FLAVOR=DVD ARCH=x86_64 \
Expand Down
Loading