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

fixes #24881 - cloud-init nocloud support #6057

Merged
merged 1 commit into from Apr 29, 2019

Conversation

timogoebel
Copy link
Member

@timogoebel timogoebel commented Sep 11, 2018

This ports the foreman_userdata plug-in to core.

Abstract

vSphere offers the functionality to clone vm from templates and customize the templates during cloning. This is very handy for changing the network settings of a newly cloned VM. Foreman fully supports this feature via a userdata template that is converted to cloud-init. The amount of things vSphere allows a user to customize are very limited, though. It's not possible to run a finish script or setup puppet with valid certificates on the new VM. This can be worked around by setting up a two step process: vSphere customization is just used to set up the network config of the host. Cloud-init is used to do the rest of the customization on first boot.

As part of this PR, Foreman should be able to serve cloud-init templates.

This leads to this workflow:

  1. An administrator creates a vSphere VM template and makes sure the cloud-init application is installed in the template VM as described below in the client setup section.
  2. Foreman creates a new cloned VM in vSphere and passes the userdata template (UserData open-vm-tools) to vSphere if the template is properly assigned to the host.
  3. vSphere changes the network settings of the new host as defined in the userdata template and boots the new VM.
  4. The VM runs cloud-init and asks Foreman for the cloud-init template (CloudInit default) if the template has been properly assinged to the host. Your VM needs network access to Foreman on ports tcp/80 and tcp/443. Cloud-init then runs the template and executes the defined actions.
  5. Cloud-init signals Foreman that the host has been built successfully.

Client setup

On RHEL7 using cloud-init from EPEL:

yum install cloud-init -y

cat << EOF > /etc/cloud/cloud.cfg.d/10_foreman.cfg
datasource_list: [NoCloud]
datasource:
  NoCloud:
    seedfrom: http://foreman.example.com/userdata/
EOF

Client debug

# Purge all cloud-init data
rm -rf /var/lib/cloud/*

# Run in foreground with debug mode enabled
/usr/bin/cloud-init -d init

Development

To test this plugin manually during development, you can request a template for a specific host by spoofing the host's IP address via a request header.

curl -D - -H 'X-FORWARDED-FOR: 192.168.1.1' http://localhost:3000/userdata/user-data

TODOs

cc: @lzap, @sean797

@lzap
Copy link
Member

lzap commented Sep 18, 2018

2. UserData open-vm-tools

Not sure what you mean by this, I mean I understand what open-vm-tools is but failing to understand context of this step. Just trying to fully understand the workflow for my testing, I barely know VMWare.

specific host by spoofing

Call the police! :-)

Allright, I am going to do initial review of the code. Thanks for the feature!

app/models/template_kind.rb Outdated Show resolved Hide resolved
Copy link
Member

@lzap lzap left a comment

Choose a reason for hiding this comment

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

Looks good.

@timogoebel
Copy link
Member Author

  1. UserData open-vm-tools

Not sure what you mean by this, I mean I understand what open-vm-tools is but failing to understand context of this step. Just trying to fully understand the workflow for my testing, I barely know VMWare.

The basic steps are:

  • You start with a template (a special kind of vm that acts as the source for cloning) in VMWare.
  • Foreman tells vmware to clone a new vm from this template. The clone request contains the rendered open-vm-tools userdata template.
  • vSphere clones the VM and applies the customizationspec to the template.
  • the new vm boots. it looks like the original template, just with the requested changes applied.

@timogoebel
Copy link
Member Author

Sorry for the delay here, I want to change the implementation quite a bit and haven't found the time yet.

Copy link
Member

@ares ares left a comment

Choose a reason for hiding this comment

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

This is great. If you don't mind, I'd suggest adding one thing for TODO section. Write a blog post or PR to our manual, with similar content you used here.

@@ -0,0 +1,39 @@
<%#
Copy link
Member

Choose a reason for hiding this comment

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

This needs to go through community templates too. I guess it helps with the review, but we should sync it from there to make sure, we have the latest version and next sync won't causd regressions.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Yes, but what i mean is that if that PR changes, we may forgot to update it in this PR too. So I think it would be better to drop it from here and after this is merged, do the sync using script/sync_templates.sh. Or do it as part of this PR but just before merge.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we should just merge the community-templates PR in sync with this PR. I like having the templates here so we automatically run snapshot & seed tests with the templates.

Copy link
Member

Choose a reason for hiding this comment

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

Let's simply agree that templates in this PR are just copies of the c-t repository, that's the master. Just avoid reviewing the templates here in this PR.

Copy link
Member

Choose a reason for hiding this comment

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

The one who merges this PR must make sure that the c-r repo PR is correct (the same content or it works).

@theforeman-bot
Copy link
Member

@timogoebel, this pull request is currently not mergeable. Please rebase against the develop branch and push again.

If you have a remote called 'upstream' that points to this repository, you can do this by running:

    $ git pull --rebase upstream develop

This message was auto-generated by Foreman's prprocessor

@timogoebel
Copy link
Member Author

Rebased, added templates to snapshot tests, dryed up the code with unattended controller.

@timogoebel timogoebel force-pushed the 24881-cloud-init-userdata branch 2 times, most recently from 6598e3c to ddcf226 Compare March 7, 2019 09:43
@timogoebel
Copy link
Member Author

Rebased. Changed templates to use dns_server helper.

lzap
lzap previously approved these changes Mar 8, 2019
Copy link
Member

@lzap lzap left a comment

Choose a reason for hiding this comment

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

This is good, I will start testing this the moment @timogoebel removes the WIP tag from the title :-) Thanks a lot of work here.

app/controllers/unattended_controller.rb Show resolved Hide resolved
app/models/template_kind.rb Outdated Show resolved Hide resolved
@@ -0,0 +1,39 @@
<%#
Copy link
Member

Choose a reason for hiding this comment

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

The one who merges this PR must make sure that the c-r repo PR is correct (the same content or it works).

@timogoebel
Copy link
Member Author

Rebased and re-synced the community-templates PR. I believe this is ready to merge.

lzap
lzap previously approved these changes Apr 25, 2019
Copy link
Member

@lzap lzap left a comment

Choose a reason for hiding this comment

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

The code looks good, all relevant PRs are ready except smart-proxy templates which needs rebase. We can merge it later on. I am currently testing the feature.

@lzap
Copy link
Member

lzap commented Apr 25, 2019

A nit pick, template kind should probably start with capital letter in the description and we should provide some text:

image

diff --git a/app/models/template_kind.rb b/app/models/template_kind.rb
index b4a504645..9cbd198be 100644
--- a/app/models/template_kind.rb
+++ b/app/models/template_kind.rb
@@ -21,7 +21,7 @@ class TemplateKind < ApplicationRecord
       "user_data" => N_("User data template"),
       "ZTP" => N_("ZTP PXE template"),
       "POAP" => N_("POAP PXE template"),
-      "cloud-init" => N_("cloud-init template")
+      "cloud-init" => N_("Cloud-init template")
     }
   end
 
@@ -36,7 +36,8 @@ class TemplateKind < ApplicationRecord
       "script" => N_("An arbitrary script, must be manually downloaded using wget/curl."),
       "user_data" => N_("Template with seed data for virtual or cloud instances when 'user data' flag is set, typically cloud-init or ignition format."),
       "ZTP" => N_("Provisioning Junos devices (Junos 12.2+)."),
-      "POAP" => N_("Provisioning for switches running NX-OS.")
+      "POAP" => N_("Provisioning for switches running NX-OS."),
+      "cloud-init" => N_("Template for cloud-init unattended endpoint.")
     }
   end

@lzap
Copy link
Member

lzap commented Apr 25, 2019

Also the name of the template reads a bit off for me, "CloudInit default". I'd probably go with "Cloud-init default", looks much nicer. Just rename the file as well.

@lzap
Copy link
Member

lzap commented Apr 25, 2019

I am getting an error when you provide an incorrect IP (non-existing host):

[app|F|74c|74c] NameError (undefined local variable or method `error_message' for #<UserdataController:0x0000555561419590>
 | Did you mean?  errors_hash):
[app|F|74c|74c]   
[app|F|74c|74c] app/controllers/concerns/foreman/controller/template_rendering.rb:28:in `render_error'
 | app/controllers/userdata_controller.rb:62:in `find_host'
 | app/controllers/concerns/foreman/controller/timezone.rb:10:in `set_timezone'
 | app/models/concerns/foreman/thread_session.rb:32:in `clear_thread'
 | app/controllers/concerns/foreman/controller/topbar_sweeper.rb:12:in `set_topbar_sweeper_controller'
 | lib/foreman/middleware/telemetry.rb:10:in `call'
 | lib/foreman/middleware/catch_json_parse_errors.rb:9:in `call'
 | lib/foreman/middleware/logging_context_session.rb:22:in `call'
 | lib/foreman/middleware/logging_context_request.rb:11:in `call'
[app|I|74c|74c]   Rendered /home/lzap/.rbenv/versions/2.5.1/lib/ruby/gems/2.5.0/gems/actionpack-5.2.1/lib/action_dispatch/middleware/templates/rescues/diagnostics.html.erb within rescues/layout (32.6ms)

@lzap
Copy link
Member

lzap commented Apr 25, 2019

Other than that it works fine.

@timogoebel
Copy link
Member Author

I am getting an error when you provide an incorrect IP (non-existing host):

Ah, this needs a test as well. Will take a look.

@timogoebel
Copy link
Member Author

This fixes the issue:

diff --git a/app/controllers/concerns/foreman/controller/template_rendering.rb b/app/controllers/concerns/foreman/controller/template_rendering.rb
index 4f9e0ba23..16662e24b 100644
--- a/app/controllers/concerns/foreman/controller/template_rendering.rb
+++ b/app/controllers/concerns/foreman/controller/template_rendering.rb
@@ -25,9 +25,9 @@ module Foreman
       end

       def render_error(options)
-        logger.error error_message % params
         message = options.delete(:message)
         status = options.delete(:status) || :not_found
+        logger.error message % options
         render plain: "#{message % options}\n", :status => status
       end
     end
diff --git a/app/models/template_kind.rb b/app/models/template_kind.rb
index b4a504645..9cbd198be 100644
--- a/app/models/template_kind.rb
+++ b/app/models/template_kind.rb
@@ -21,7 +21,7 @@ class TemplateKind < ApplicationRecord
       "user_data" => N_("User data template"),
       "ZTP" => N_("ZTP PXE template"),
       "POAP" => N_("POAP PXE template"),
-      "cloud-init" => N_("cloud-init template")
+      "cloud-init" => N_("Cloud-init template")
     }
   end

@@ -36,7 +36,8 @@ class TemplateKind < ApplicationRecord
       "script" => N_("An arbitrary script, must be manually downloaded using wget/curl."),
       "user_data" => N_("Template with seed data for virtual or cloud instances when 'user data' flag is set, typically cloud-init or ignition format."),
       "ZTP" => N_("Provisioning Junos devices (Junos 12.2+)."),
-      "POAP" => N_("Provisioning for switches running NX-OS.")
+      "POAP" => N_("Provisioning for switches running NX-OS."),
+      "cloud-init" => N_("Template for cloud-init unattended endpoint.")
     }
   end

diff --git a/test/controllers/userdata_controller_test.rb b/test/controllers/userdata_controller_test.rb
index 1efb82450..57f619c68 100644
--- a/test/controllers/userdata_controller_test.rb
+++ b/test/controllers/userdata_controller_test.rb
@@ -63,6 +63,15 @@ class UserdataControllerTest < ActionController::TestCase
         assert_response :success
         assert_equal user_data_content, @response.body
       end
+
+      context 'with unknown ip address' do
+        test 'should display an error' do
+          @request.env['REMOTE_ADDR'] = '198.51.100.1'
+          get :userdata
+          assert_response :not_found
+          assert_includes @response.body, 'Could not find host for request 198.51.100.1'
+        end
+      end
     end

     context 'with cloud-init template' do

@timogoebel
Copy link
Member Author

@lzap: Tests passed.

@lzap
Copy link
Member

lzap commented Apr 29, 2019

Please rebase.

@timogoebel
Copy link
Member Author

Rebased.

Copy link
Member

@lzap lzap left a comment

Choose a reason for hiding this comment

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

Works fine, thank you so much for doing the extra work and pushing this into core. Katello tests did not run however this provisioning endpoint is very unlikely to affect Katello in any way.

@lzap lzap merged commit d74dc42 into theforeman:develop Apr 29, 2019
@lzap
Copy link
Member

lzap commented Apr 29, 2019

Merged, thanks. Feel free to turn the description into manual and send it to manual site and/or tutorial section of our discourse.

@ares
Copy link
Member

ares commented Apr 30, 2019

Awesome work! I'm happy to see this in core. Also I'm looking forward for the documentation update :-)

@timogoebel
Copy link
Member Author

The only thing still missing is the possibility to create a template from an existing host. I believe we already have all the bits and pieces, we just have to glue them together.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
9 participants