[provisioner] Add autoyast self update option #823

nicolasbock · 2016-11-23T20:25:56Z

AutoYaST can update itself before installing the OS. This patch enables
this feature and adds a field to the provisioner barclamp so that a
repository URL can be set by the user. The special token <ADMINWEB> resolves
to the IP:port of the admin node. An empty URL defaults to the SUSE
customer center (scc.suse.com).

vuntz · 2016-11-25T16:28:22Z

chef/data_bags/crowbar/template-provisioner.json

@@ -9,6 +9,8 @@
      "access_keys": "",
      "shell_prompt": "USER@HOST:CWD SUFFIX",
      "enable_pxe": true,
+      "self_update": false,
+      "self_update_url": "http://updates.suse.com/sle12/$arch",


Shouldn't we default this to a repo that is mirrored on the admin server, like all the other repos we have there?

Also, I'm not convinced we need a boolean to enable this or not. Either the repo is present locally (we enable), or the user provides an external URL (we enable), or the repo is not present locally and there's no url (we disable)

@vuntz Thanks. I changed the PR.

vuntz · 2016-11-28T10:13:44Z

chef/cookbooks/provisioner/templates/default/autoyast.xml.erb

@@ -26,6 +26,7 @@
      <import_gpg_key config:type="boolean">true</import_gpg_key>
    </signature-handling>
    <storage/>
+    <self_update_url><%= @self_update_url %></self_update_url>


Didn't we want to add some unless @self_update_url.nil? || @self_update_url.empty?

done. Now, if the URL is empty AutoYaST will default to scc.suse.com if it's there.

vuntz · 2016-11-28T10:16:29Z

chef/data_bags/crowbar/template-provisioner.json

@@ -9,6 +9,7 @@
      "access_keys": "",
      "shell_prompt": "USER@HOST:CWD SUFFIX",
      "enable_pxe": true,
+      "self_update_url": "http://crowbar/sle12/$arch",


I would change the default to: http://<ADMINWEB>/suse-12.2/$arch/repos/installer-update (or something similar).

A couple of things:

<ADMINWEB> should be substituted in the rails app (see glance & tempest barclamps)

if we put suse-12.2 in the path, then maybe that attribute should live in a versioned suse attribute path somewhere (we already have that in the provisioner barclamp, I think)

we need a doc bug to document how to mirror that repo in the location

I've add the <ADMINWEB> alias.

Opened doc bug: https://bugzilla.suse.com/show_bug.cgi?id=1012469

I reverted the default to "" which defaults to scc.suse.com. Or would you prefer a versioned local URL?

vuntz · 2016-11-28T10:16:38Z

crowbar_framework/app/views/barclamp/provisioner/_edit_attributes.html.haml

@@ -7,6 +7,9 @@
    %span.help-block
      = t(".access_keys_hint")

+    = boolean_field :self_update


This one is gone :-)

Good catch! I removed it.

houndci-bot · 2016-11-28T20:35:57Z

chef/cookbooks/provisioner/recipes/update_nodes.rb

@@ -333,6 +334,8 @@ def find_node_boot_mac_addresses(node, admin_data_net)
                      web_port: web_port,
                      packages: packages,
                      repos: repos,
+                      self_update_url: node[:provisioner][:self_update_url].gsub(
+                        /<ADMINWEB>/, "#{admin_ip}:#{web_port}"),


Style/MultilineMethodCallBraceLayout: Closing method call brace must be on the line after the last argument when opening brace is on a separate line from the first argument.

vuntz · 2016-11-29T09:09:42Z

crowbar_framework/config/locales/provisioner/en.yml

@@ -21,6 +21,7 @@ en:
      edit_attributes:
        access_keys: 'Additional SSH keys'
        access_keys_hint: 'Each SSH key must be on its own line'
+        self_update_url: 'AutoYaST Self-update URL (The alias <ADMINWEB> can be used to specify the admin node)'


s/admin node/admin server/

vuntz · 2016-11-29T09:10:05Z

chef/data_bags/crowbar/template-provisioner.json

@@ -9,6 +9,7 @@
      "access_keys": "",
      "shell_prompt": "USER@HOST:CWD SUFFIX",
      "enable_pxe": true,
+      "self_update_url": "",


So we don't provide a default value that just works? :-)

:) Well, the default in this case is to use scc.suse.com which works. But we could also use a crowbar local default. Which would you prefer?

I think the default should be the mirror on the admin server.

vuntz · 2016-11-29T09:10:37Z

chef/cookbooks/provisioner/recipes/update_nodes.rb

@@ -290,6 +290,7 @@ def find_node_boot_mac_addresses(node, admin_data_net)
          append << "ifcfg=dhcp4 netwait=60"
          append << "squash=0" # workaround bsc#962397
          append << "autoupgrade=1" if mnode[:state] == "os-upgrading"
+          append << "self_update"


Shouldn't we pass this only if self_update_url is not empty?

Also, if the URL contains <ADMINWEB>, I think we could do some quick check with File.exists? so that if we use a default with <ADMINWEB> but the repo is not mirrored, we just don't enable the update feature.

I added settings to autoyast.xml so that failures in self update don't mean failure anymore. AutoYaST will log those failure but continue with the installation process.

I'll add a check whether the repository is actually there.

I guess the check you mention here is still not there? :-)

I added it now.

houndci-bot · 2016-11-30T20:48:39Z

crowbar_framework/app/models/provisioner_service.rb

+    return if all_nodes.empty?
+
+    nodes = NodeObject.find("roles:provisioner-server")
+    unless nodes.nil? or nodes.length < 1


Style/AndOr: Use || instead of or. (https://github.com/bbatsov/ruby-style-guide#no-and-or-or)
Style/ZeroLengthPredicate: Use empty? instead of length < 1.

houndci-bot · 2016-11-30T20:48:40Z

crowbar_framework/app/models/provisioner_service.rb

+    #      "#{proposal["attributes"]["provisioner"]["self_update_url"]}"
+    #    ))
+    #  end
+    #end


Style/LeadingCommentSpace: Missing space after #. (https://github.com/bbatsov/ruby-style-guide#hash-space)

houndci-bot · 2016-11-30T20:48:41Z

crowbar_framework/app/models/provisioner_service.rb

@@ -50,6 +51,21 @@ def validate_proposal_after_save proposal
      end
    end

+    @logger.info("validating existence of #{proposal["attributes"]["provisioner"]["self_update_url"]}")
+
+    #if /<ADMINWEB>/ =~ proposal["attributes"]["provisioner"]["self_update_url"]


Style/LeadingCommentSpace: Missing space after #. (https://github.com/bbatsov/ruby-style-guide#hash-space)

houndci-bot · 2016-11-30T20:48:41Z

crowbar_framework/app/models/provisioner_service.rb

@@ -50,6 +51,21 @@ def validate_proposal_after_save proposal
      end
    end

+    @logger.info("validating existence of #{proposal["attributes"]["provisioner"]["self_update_url"]}")


Metrics/LineLength: Line is too long. [103/100] (https://github.com/SUSE/style-guides/blob/master/Ruby.md#metricslinelength)

nicolasbock · 2016-11-30T22:08:53Z

@vuntz Here is where I am at with this PR:

The self update feature is always enabled now
I added some settings to the autoyast file so that a missing or unavailable self update URL is not fatal. A warning is shown and after 10 seconds the installation process continues. The warning is logged and will show up in a supportconfig.
The default URL points to a mirrored repository on the admin server
We don't verify that that mirror exists and instead rely on the logged error message from AutoYaST. To verify we need to know the http server root directory and replace the proper parts of the URI with that directory. Since we don't know what arch we will deploy the role on, we have to be happy with any $arch and testing for that seemed more than a line of code. Of course I might very well not seeing the simple solution here, so please correct me if I am making this too complicated 😉.
An empty URL means update from scc.suse.com
I still don't fully understand how the defaults from a role end up in a node's attributes. This PR replaces the <ADMINWEB> shortcut in the chef recipe. I tried to follow the glance barclamp but couldn't get it to work here. i.e. if I replace <ADMINWEB> in apply_role_pre_chef_call in models/provider_service.rb the replace URI does not end up in the node.

rsalevsky · 2016-12-02T10:33:21Z

@nicolasbock Can you please change the prefix from [provisioner] to provisioner:?

nicolasbock · 2016-12-02T12:36:23Z

@rsalevsky oop, sorry, of course.

aspiers

No commit message and no PR description => automatic -1 ;-p

nicolasbock · 2016-12-13T21:11:46Z

@aspiers I have added a description to the commit. Could you have another look?

vuntz · 2017-01-06T13:03:44Z

chef/data_bags/crowbar/template-provisioner.schema

@@ -19,6 +19,7 @@
            "access_keys": { "type": "str", "required": true },
            "shell_prompt": { "type": "str", "required": true },
            "enable_pxe": { "type": "bool", "required": true },
+            "self_update_url": { "type": "str", "required": true },


Sorry, I failed to notice this earlier on, but we have a suse attribute, and this should live in there. See line 56.

vuntz · 2017-01-06T13:05:14Z

chef/cookbooks/provisioner/recipes/update_nodes.rb

@@ -290,6 +290,7 @@ def find_node_boot_mac_addresses(node, admin_data_net)
          append << "ifcfg=dhcp4 netwait=60"
          append << "squash=0" # workaround bsc#962397
          append << "autoupgrade=1" if mnode[:state] == "os-upgrading"
+          append << "self_update"


I guess the check you mention here is still not there? :-)

houndci-bot · 2017-01-10T14:10:04Z

chef/cookbooks/provisioner/recipes/update_nodes.rb

+      do_self_update = node[:provisioner][:suse][:autoyast][:do_self_update]
+      if do_self_update && ! expanded_self_update_url.empty?
+        do_self_update = system("wget --quiet --spider #{expanded_self_update_url.gsub("$arch", "#{arch}")}")
+        if ! do_self_update


Metrics/BlockNesting: Avoid more than 3 levels of block nesting. (https://github.com/bbatsov/ruby-style-guide#three-is-the-number-thou-shalt-count)
Style/NegatedIf: Favor unless over if for negative conditions. (https://github.com/bbatsov/ruby-style-guide#unless-for-negatives)
Style/SpaceAfterNot: Do not leave space between ! and its argument. (https://github.com/bbatsov/ruby-style-guide#no-space-bang)

houndci-bot · 2017-01-10T14:10:05Z

chef/cookbooks/provisioner/recipes/update_nodes.rb

+      )
+      do_self_update = node[:provisioner][:suse][:autoyast][:do_self_update]
+      if do_self_update && ! expanded_self_update_url.empty?
+        do_self_update = system("wget --quiet --spider #{expanded_self_update_url.gsub("$arch", "#{arch}")}")


Style/UnneededInterpolation: Prefer to_s over string interpolation.
Metrics/LineLength: Line is too long. [109/100] (https://github.com/SUSE/style-guides/blob/master/Ruby.md#metricslinelength)

houndci-bot · 2017-01-10T14:10:05Z

chef/cookbooks/provisioner/recipes/update_nodes.rb

+        "<ADMINWEB>", "#{admin_ip}:#{web_port}"
+      )
+      do_self_update = node[:provisioner][:suse][:autoyast][:do_self_update]
+      if do_self_update && ! expanded_self_update_url.empty?


Style/SpaceAfterNot: Do not leave space between ! and its argument. (https://github.com/bbatsov/ruby-style-guide#no-space-bang)

houndci-bot · 2017-01-10T14:14:09Z

chef/cookbooks/provisioner/recipes/update_nodes.rb

+      if do_self_update && !expanded_self_update_url.empty?
+        do_self_update = system("wget --quiet --spider " +
+                                "#{expanded_self_update_url.gsub("$arch", "#{arch}")}")
+        unless do_self_update


Metrics/BlockNesting: Avoid more than 3 levels of block nesting. (https://github.com/bbatsov/ruby-style-guide#three-is-the-number-thou-shalt-count)

houndci-bot · 2017-01-10T14:14:09Z

chef/cookbooks/provisioner/recipes/update_nodes.rb

+      do_self_update = node[:provisioner][:suse][:autoyast][:do_self_update]
+      if do_self_update && !expanded_self_update_url.empty?
+        do_self_update = system("wget --quiet --spider " +
+                                "#{expanded_self_update_url.gsub("$arch", "#{arch}")}")


Style/UnneededInterpolation: Prefer to_s over string interpolation.

houndci-bot · 2017-01-10T14:14:10Z

chef/cookbooks/provisioner/recipes/update_nodes.rb

+      )
+      do_self_update = node[:provisioner][:suse][:autoyast][:do_self_update]
+      if do_self_update && !expanded_self_update_url.empty?
+        do_self_update = system("wget --quiet --spider " +


Style/LineEndConcatenation: Use \ instead of + or << to concatenate those strings.

houndci-bot · 2017-01-10T14:15:52Z

chef/cookbooks/provisioner/recipes/update_nodes.rb

+      do_self_update = node[:provisioner][:suse][:autoyast][:do_self_update]
+      if do_self_update && !expanded_self_update_url.empty?
+        do_self_update = system("wget --quiet --spider " \
+                                expanded_self_update_url.gsub("$arch", "#{arch}"))


unexpected token tIDENTIFIER
unexpected token tRPAREN

nicolasbock · 2017-01-16T16:12:49Z

@aspiers I have added a commit message and a PR comment.

nicolasbock · 2017-01-16T16:14:25Z

@vuntz I think I have addressed the issues you raised. Could you have another look?

AutoYaST can update itself before installing the OS. This patch enables this feature and adds a field to the provisioner barclamp so that a repository URL can be set by the user. The special token `<ADMINWEB>` resolves to the IP:port of the admin node. An empty URL defaults to the SUSE customer center (scc.suse.com).

Complaint addressed

aspiers · 2017-01-25T15:15:36Z

@nicolasbock Removed my -1. I can't give +1 because I don't even understand why autoyast would need to self-update :-) If there's newer autoyast code then why wouldn't it be simply provided via TFTP/HTTP the first time around?

nicolasbock · 2017-01-25T16:38:15Z

@aspiers Thanks! Regarding your question: I haven't checked but am guessing that the autoyast comes from the image pulled from tftp on the admin node. I don't know though how and where from it is updated and I think your question is very valid 😄.

I forgot to add this Trello card.

nicolasbock · 2017-03-10T19:16:42Z

@vuntz Could you have another look, please?

nicolasbock added the wip label Nov 23, 2016

nicolasbock force-pushed the installer_update branch 3 times, most recently from b3caa83 to 910de87 Compare November 24, 2016 13:49

vuntz suggested changes Nov 25, 2016

View reviewed changes

nicolasbock force-pushed the installer_update branch from 910de87 to 05b21f5 Compare November 26, 2016 13:18

vuntz reviewed Nov 28, 2016

View reviewed changes

nicolasbock force-pushed the installer_update branch from 05b21f5 to 8f70db2 Compare November 28, 2016 20:35

houndci-bot reviewed Nov 28, 2016

View reviewed changes

nicolasbock force-pushed the installer_update branch from 8f70db2 to df35e12 Compare November 28, 2016 20:45

nicolasbock removed the wip label Nov 28, 2016

vuntz suggested changes Nov 29, 2016

View reviewed changes

nicolasbock force-pushed the installer_update branch from df35e12 to 011cbf1 Compare November 30, 2016 20:48

houndci-bot reviewed Nov 30, 2016

View reviewed changes

nicolasbock force-pushed the installer_update branch 2 times, most recently from 3e5c773 to d719711 Compare November 30, 2016 21:53

nicolasbock force-pushed the installer_update branch from d719711 to b9582db Compare December 2, 2016 12:37

aspiers previously requested changes Dec 12, 2016

View reviewed changes

nicolasbock force-pushed the installer_update branch from b9582db to f3afca5 Compare December 13, 2016 21:11

vuntz suggested changes Jan 6, 2017

View reviewed changes

nicolasbock force-pushed the installer_update branch from f3afca5 to 08ae4fc Compare January 10, 2017 14:09

houndci-bot reviewed Jan 10, 2017

View reviewed changes

nicolasbock force-pushed the installer_update branch 2 times, most recently from 69174ee to e0eb5d2 Compare January 10, 2017 14:14

houndci-bot reviewed Jan 10, 2017

View reviewed changes

nicolasbock force-pushed the installer_update branch from e0eb5d2 to 19d6255 Compare January 10, 2017 14:15

houndci-bot reviewed Jan 10, 2017

View reviewed changes

nicolasbock force-pushed the installer_update branch from 19d6255 to e045823 Compare January 10, 2017 14:16

nicolasbock force-pushed the installer_update branch from e045823 to ffaef28 Compare January 23, 2017 19:18

dirkmueller modified the milestones: Cloud 7 Update1, Cloud 7 Update2 Jan 27, 2017

jsuchome added the needs backport to SOC7 (stable/4.0) label Jun 15, 2017

dirkmueller removed the needs backport to SOC7 (stable/4.0) label Oct 5, 2017

JanZerebecki added the wip label Jan 13, 2021

[provisioner] Add autoyast self update option #823

Are you sure you want to change the base?

[provisioner] Add autoyast self update option #823

Conversation

nicolasbock commented Nov 23, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolasbock commented Nov 30, 2016

rsalevsky commented Dec 2, 2016

nicolasbock commented Dec 2, 2016

aspiers left a comment

Choose a reason for hiding this comment

nicolasbock commented Dec 13, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolasbock commented Jan 16, 2017

nicolasbock commented Jan 16, 2017

aspiers commented Jan 25, 2017

nicolasbock commented Jan 25, 2017

nicolasbock commented Mar 10, 2017

nicolasbock commented Nov 23, 2016 •

edited

Loading