cloud-init / ubuntu-sru

3 stars 7 forks source link

RFC: Instead of generating manual scripts from templates, implement them as a static script #69

Open OddBloke opened 4 years ago

OddBloke commented 4 years ago

Currently, we generate the manual scripts required for each SRU from the templates in sru-templates/manual. The generation happens into manual/, and we then run those generated scripts.

This process was originally put in place, I believe, when the expectation was that we would modify those generated scripts to include additional validation specific to this SRU. However, I don't believe we have used them this way in the past few SRU cycles.

I propose that we modify the SRU process to have those scripts be static, so that we aren't effectively maintaining two copies (the template and the generated script) in the course of each SRU cycle.

We would need to handle the two templated variables that are currently used, %SRU_SERIES% and %SRU_LP_USER%.

For %SRU_LP_USER%, I propose that we modify the manual scripts to take it as a parameter, so an invocation would look like: sru-templates/manual/gce-sru daniel-thewatkins.

For %SRU_SERIES%, a parameter is a little less convenient (because passing arrays in shell is Not Fun). One potential solution is to hard-code the value of SRU_SERIES in each manual template; this set changes infrequently, so I believe it would carry little maintenance cost. A more radical alternative would be to modify the manual scripts to take one series as a parameter, with the expectation that the script would be called once for each SRU series. This has the advantage of simplifying the scripts, and making running SRU verification in parallel much easier.

(For GCE verification, I have already been manually editing the SRU_SERIES variable so that I can invoke the script more than once in parallel to speed things up.)

raharper commented 4 years ago

I'm +1 on this approach, though I think in some cases there are still some "per-user" configurations, such as security groups or other values. Could we specify templated input file that we'd keep out of tree? and then just pass in this additional config file, were we could source the per-cloud specific values?

blackboxsw commented 4 years ago

This is a sound proposal to me. I've been thinking the same, a lot of duplication and we haven't been frequently extending the manual script often enough for each SRU to make the template base worth while. I propose that if we have specific tests that need to be validatied for a given SRU we just capture those additions in between some sort of bookend comment that we can quickly understand to drop on next SRU if inapplicable.

# BEGIN Specific SRU feature tests
...
# END Specific SRU feature tests

Then we can drop those tests in the next SRU if needed.

blackboxsw commented 4 years ago

I'm +1 on this approach, though I think in some cases there are still some "per-user" configurations, such as security groups or other values. Could we specify templated input file that we'd keep out of tree? and then just pass in this additional config file, were we could source the per-cloud specific values?

I like the idea of a shell config file that is sourced. which sets all user-specific values we care about.

OddBloke commented 4 years ago

I think in some cases there are still some "per-user" configurations, such as security groups or other values

None of those are templated currently (see below), but I agree that we should capture them somehow. A source'd config file seems like a very reasonable way of capturing that.

Regarding templating,

for cloud in sru-templates/manual/*; do
  diff -u $cloud manual/$(basename $cloud)-19.3.41.txt
done

produces

--- sru-templates/manual/azure-sru  2019-09-25 15:13:12.037538177 -0400
+++ manual/azure-sru-19.3.41.txt    2019-12-06 16:35:02.596094729 -0500
@@ -3,7 +3,7 @@
 # Check Tracebacks or error conditions on clean boot

 # To be adapted to the SRU to test
-SRU_SERIES="%SRU_SERIES%"
+SRU_SERIES="xenial bionic disco eoan"

 # local values
 # find region with az account list-locations -o table
@@ -18,7 +18,7 @@
 cat > sethostname.yaml <<EOF
 ## template: jinja
 #cloud-config
-ssh_import_id : [%SRU_LP_USER%]
+ssh_import_id : [daniel-thewatkins]
 hostname: SRU-worked-{{v1.cloud_name}}
 EOF

--- sru-templates/manual/ec2-sru    2019-09-04 10:00:53.158678104 -0400
+++ manual/ec2-sru-19.3.41.txt  2019-12-06 16:35:02.596094729 -0500
@@ -2,10 +2,10 @@

 set -e

-SRU_SERIES="%SRU_SERIES%"
+SRU_SERIES="xenial bionic disco eoan"

 # local values
-LP_USER=${LP_USER:-"%SRU_LP_USER%"}
+LP_USER=${LP_USER:-"daniel-thewatkins"}

 # Provide USE_DEV_PPA=1 if we are SRU queue is still unapproved
 # This will use our ppa:cloud-init-dev/proposed
--- sru-templates/manual/gce-sru    2019-12-09 14:51:13.942918103 -0500
+++ manual/gce-sru-19.3.41.txt  2019-12-06 17:35:20.305958618 -0500
@@ -1,13 +1,13 @@
 # Manual GCE upgrade and clean install validation Xenial, Bionic, Cosmic

 # To be adapted to the SRU to test
-SRU_SERIES="%SRU_SERIES%"
+SRU_SERIES="eoan"
 ZONE="us-central1-b"

 cat > sethostname.yaml << EOF
 ## template: jinja
 #cloud-config
-ssh_import_id : [%SRU_LP_USER%]
+ssh_import_id : [daniel-thewatkins]
 hostname: SRU-worked-{{v1.cloud_name}}
 EOF

--- sru-templates/manual/openstack-sru  2019-09-04 10:00:53.158678104 -0400
+++ manual/openstack-sru-19.3.41.txt    2019-12-06 16:35:02.604094830 -0500
@@ -1,9 +1,9 @@
 # Manual OpenStack upgrade and clean install validation Xenial, Bionic, Cosmic

-SRU_SERIES="%SRU_SERIES%"
+SRU_SERIES="xenial bionic disco eoan"

 # local values
-LP_USER="%SRU_LP_USER%"
+LP_USER="daniel-thewatkins"
 ADMIN_NET_ID=""
 SSH_KEY=""

--- sru-templates/manual/oracle-sru 2019-09-25 15:13:12.037538177 -0400
+++ manual/oracle-sru-19.3.41.txt   2019-12-06 16:35:02.604094830 -0500
@@ -1,9 +1,9 @@
 # Manual Oracle upgrade and clean install validation

-SRU_SERIES="%SRU_SERIES%"
+SRU_SERIES="xenial bionic disco eoan"

 # local values
-LP_USER="%SRU_LP_USER%"
+LP_USER="daniel-thewatkins"

 cat > sethostname.yaml << EOF
 ## template: jinja
--- sru-templates/manual/softlayer-sru  2019-09-25 15:13:12.037538177 -0400
+++ manual/softlayer-sru-19.3.41.txt    2019-12-06 16:35:02.604094830 -0500
@@ -18,7 +18,7 @@
 set -x

 # local values defaults set by sru-create.py
-SRU_SERIES=${SRU_SERIES:-"%SRU_SERIES%"}
+SRU_SERIES=${SRU_SERIES:-"xenial bionic disco eoan"}
 LP_USER=${LP_USER:-"%LP_USER%"}

 # Provide USE_DEV_PPA=1 if we are SRU queue is still unapproved
--- sru-templates/manual/vmware-sru 2019-09-04 10:00:53.158678104 -0400
+++ manual/vmware-sru-19.3.41.txt   2019-12-06 16:35:02.604094830 -0500
@@ -4,7 +4,7 @@
 commands.

 # To be adapted to the SRU to test
-SRU_SERIES="%SRU_SERIES%"
+SRU_SERIES="xenial bionic disco eoan"

 ## xenial

@@ -74,7 +74,7 @@
 cat > sethostname.yaml << EOF
 ## template: jinja
 #cloud-config
-ssh_import_id : [%SRU_LP_USER%]
+ssh_import_id : [daniel-thewatkins]
 hostname: SRU-worked-{{v1.cloud_name}}
 EOF

so we can see that there isn't much difference. (Also note that the LP user ended up being mine even though I wasn't the one who performed the generation; this new approach would avoid that also.)

blackboxsw commented 4 years ago

yes the generated doesn't have much difference. As a separate task I think we should use a common header that source the 'config' file for expected user-specific settings, as openstack and azure both have a number of manual customizations required even despite the generated sru test file.