-
Notifications
You must be signed in to change notification settings - Fork 12
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 playbook for openqa pt install #247
Conversation
97561bf
to
d6b2ee6
Compare
|
||
- name: Extract the first part of az_container_name | ||
set_fact: | ||
az_container_top: "{{ az_container_name.split('/')[0] }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can avoid this "trick" (that create hidden dependency of path used in hana_media.yaml and playbooks/sap-hana-download-media.yaml) and just force the user to provide the path in a second variable just for this playbook
ptf_installation.yaml -e ptf_container="aaa/bbb/ptf" -e ptf_files="aaa.rpm,bbb.rpm"
or with the path in each file element
ptf_installation.yaml -e ptf_files="aaa/bbb/ptf/aaa.rpm,aaa/bbb/ptf/bbb.rpm"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that can be done - but is it really needed? This code just grabs the first part, which is always bound to be the name of the top-level container, which isn't going to change (or, if it changes, it will change for everything). Let's discuss in slack
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can decide to use a dedicated container for PTF.
Maybe one with a subfolder for each PTF
005142d
to
d7ceafc
Compare
ptf_url: "your_ptf_url" | ||
ptf_user: "your_username_here" | ||
ptf_password: "your_pass_here" | ||
url_timeout: 30 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why we like them to be parametric?
why hardcoding value directly in Download PTF files with SAS token
is not enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
My only concern is that it come out with a complicated API (no idea if /how we can improve it)
url_timeout
,url_retries_cnt
,url_retries_delay
can be configured from the external but has default. There's no validatio nof values if configured from the external.sas_token
is optional. Its presence is one of the variables checked to decide which download task to use. Name overlap with the one used in hana_media.yaml but they are two different variables (even if for the moment, due to what we currently have in Azure, token is the same). It cold be an idea to rename it toptf_sas_token
and just externally (openQA) use same token used for HANA install?ptf_files
is mandatory ifsas_token
is defined. It has to have a special format like comma separated list of filename (or is filename + some path?). Its presence is not checked or validated before to start using itcontainer
,storage
are mandatory ifsas_token
is defined. They have to be strings. Their presence are not checked or validated before to start using themptf_user
,ptf_password
,ptf_url
are optional and used to select a specific download task. The first two are string, password is a secret, ptf_url is an url. None are validated.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This pr adds
one playbook to download and install a ptf in the nodes, both manually (via the suse ptf download directory) and on openqa (via azure storage)
one playbook for specific tasks needed by the azure fence agents
Related ticket: https://jira.suse.com/browse/TEAM-9453
Verification run: https://openqa.suse.de/tests/14729087 (failure later and unrelated)
https://openqa.suse.de/tests/14744852 (newest)
https://openqa.suse.de/tests/14744947 (without ptf)