Skip to content
This repository has been archived by the owner on Sep 26, 2021. It is now read-only.

RedHat provisioning #1090

Merged
merged 39 commits into from
May 26, 2015
Merged

RedHat provisioning #1090

merged 39 commits into from
May 26, 2015

Conversation

ehazlett
Copy link
Contributor

@ehazlett ehazlett commented May 1, 2015

This enables Machine to provision RedHat (RHEL). Tested on RHEL 7.0.

Refs #905

@ehazlett ehazlett added this to the 0.3.0 milestone May 1, 2015
return nil
}

func (provisioner *RedHatProvisioner) isAWS() bool {
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 wasn't sure about this. On AWS the extras have to be configured with a separate command. Perhaps there is a better way.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh really? Very interesting. We should ping Nirmal and see what he thinks.

Also, what is this endpoint? We should probably at least put a comment saying what it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you are using RHEL with a subscription you can use the Docker docs to install. However, on EC2, that command doesn't work -- you have to use the above. I'm assuming it's subscription related.

@ehazlett
Copy link
Contributor Author

ehazlett commented May 1, 2015

// the extras repo different than a standalone rhel box

repoCmd := "subscription-manager repos --enable=rhel-7-server-extras-rpms"
if provisioner.isAWS() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not check whether we're on AWS by attempting a typecast of the driver to amazonec2.Driver like so:

if _, err := provisioner.Driver.(amazonec2.Driver); err != nil {
    // do AWS-specific logic
} else {
    // do logic for everyone else
}

Admittedly this won't work for generic, so maybe we could fall back on the other method in that case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I think the typecast is better. Thx!

Copy link
Contributor

Choose a reason for hiding this comment

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

How do we deal with RHEL7 like distros?

@ehazlett ehazlett force-pushed the redhat-provisioning branch from d54e8b1 to 6ca06a7 Compare May 1, 2015 20:36

// request tty -- fixes error with hosts that use
// "Defaults requiretty" in /etc/sudoers - I'm looking at you RedHat
if err := session.RequestPty("xterm-256color", termHeight, termWidth, modes); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

we should set the terminal to os.Getenv("TERM") and default to xterm

@ibuildthecloud
Copy link
Contributor

I just put in #1096. It's for RancherOS support, but as a part of that work I refactored the ubuntu provisioner out into a GenericProvisioner and UbuntuProvision. Seem like this code could also embed the GenericProvisioner I created.

@@ -999,6 +999,21 @@ Options:

The DigitalOcean driver will use `ubuntu-14-04-x64` as the default image.

#### exoscale
Copy link
Contributor

Choose a reason for hiding this comment

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

For alphabetical order, or...?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it was listed out of order from one of the rebases.

@RusDavies
Copy link

Hi,

I few days ago, I started coding a FedoraProvisioner for docker-machine. Then I realized this pull request for a (very similar) Redhat provisioner was already in progress. However, there are some differences.

So, I adapted by using the code for this pull request (from the redhat-provisioning branch of ehazlett/machine), and modifying it to allow integration of my code for Fedora. To do so, I split the RedhatProvisioner into two parts, one containing code that is generic to all redhat-family distributions, and another that seems specific to RHEL only. With that done, it was a trivial matter to add a new Fedora provisioner, which others may find useful.

Code is here https://github.com/RusDavies/machine/tree/provisioner-fedora. Important parts are the files redhat_family.go, redhat_family_RHEL.go, and redhat_family_fedora.go, all under libmachine/provision.

I've been using the resulting FedoraProvisioner on Amazon E2, using the standard Fedora 21 cloud images. It seems to works fine. Clearly, however, more thorough testing is required.

At this point I'd like to engage in discussion about the acceptability of this adaptation/extension of the RedhatProvisioner code for inclusion in docker-machine. However, given the RedhatProvisioner code itself hasn't yet been merged, then I'm not sure of the best way to proceed. Which of the following is preferable:

  1. Make a new pull request against the ehazlett/machine repository
  2. Make a new pull request against the docker/machine repository
  3. Wait for the RedhatProvisioner to be merged, and then make a pull request against docker/machine.
  4. Something else?

Thanks :o)

@sthulb
Copy link
Contributor

sthulb commented May 12, 2015

@ehazlett at this point, can this provisioner be a generic for both redhat + centos + fedora? or are they very different?

@RusDavies
Copy link

@sthulb , there are some items in this RedhatProvisioner that seem very specific to RHEL only, such as the configureRepos(). Likewise for isAWS(), and installOfficialDocker(). Usage of these RHEL specific methods are hard coded into the Provision() procedure.

However, the differences between RHEL and Fedora seem minor. If a suitable interface is provided, that can be assigned in an embedding structure, thereby allowing virtual dispatch overrides, then those differences can be lifted out into overriding methods in the embedding structure that are called at specific points in the provisioning process. Makes it nice and DRY. That's what I've done in my code (see comment before yours).

@ehazlett
Copy link
Contributor Author

@RusDavies i have actually been working on Fedora and CentOS provisioners based off of RHEL.

@sthulb yeah there are some minor differences.

@RusDavies you can see the branch here: ehazlett/machine@redhat-provisioning...ehazlett:fedora-provisioner

It's pretty lightweight.

@RusDavies
Copy link

@ehazlett , I thought I may be too late to the game. Good to know it's coming, anyway. :o)

Some questions:

  1. How will you prevent the FedoraProvisioner from attempting to run your RHEL subscription stuff. Will you gate it only for OsReleaseId="rhel"? For example, this will fail on Fedora: "subscription-manager repos --enable=rhel-7-server-extras-rpms". Similarly, what's the effect on Fedora of, "yum-config-manager --enable rhui-REGION-rhel-server-extras"?

  2. What other configuration nuances will other redhat-like distributions have? Are you going to include every nuance in the general code, and gate each case? Is it not better to take such distribution specifics out of the generalized code, and put them in a higher layer?

Thanks

UPDATE: Just tried the commands in configureRepos() on a Fedora machine:

  • "subscription-manager repos --enable=rhel-7-server-extras-rpms" fails with an error: command not found.
  • "yum-config-manager --enable rhui-REGION-rhel-server-extras" does nothing, and returns without error.

Thus, FedoraProvisioner will work if isAWS() returns true, but will fail otherwise. So, at least, maybe gate the call to configureRepos()?

@ehazlett
Copy link
Contributor Author

@RusDavies thanks for the feedback. It's not done (I've experienced the same issues in Fedora as well) but I pushed to show the work. It most likely won't make it in for 0.3.0 as we have a bunch of testing for RHEL yet.

The problem with the RedHat distros is each one configures things slightly different then with various repos. This multiplies when you add more flavors.

@RusDavies
Copy link

@ehazlett , no problem. I'm a happy to help.

The slight variance between Redhat friends is precisely my concern. As support for other friends are added, it may tend towards a spaghetti of gates and exceptions.

I've updated my own code to more closely match your branch with the FedoraProvisioner. Take another look: using an interface, as I have, to place hooks in the process really doesn't add much complexity, but does help to create separation between distribution specifics.

Oh, and one other thing... given my use-cases tend to involve compliance concerns, seeing a "--nogpgcheck" on a yum command makes me frown just a little ;o)

@ehazlett
Copy link
Contributor Author

@RusDavies actually after looking we don't need configureRepo at all since we use our own RPMs. I will look into yours but I'm not sure we need all of the hooks -- the Fedora provisioner is now 39 loc. I would lean towards adding the hooks only if needed as it's more overhead and potential code paths.

@RusDavies
Copy link

@ehazlett , ok, fair enough.

@RusDavies
Copy link

@ehazlett, more feedback ...

RedhatProvisioner.installOfficialDocker() fails if docker is already installed from a different source.

This can happen when using "--driver generic" to connect to an existing machine, or otherwise using a custom image, in either case where docker may already be installed from e.g. distribution repos.

Worth noting, at least in my case, and likely for others, machines are deployed against a BOM which includes software versions. If I have an AMI that includes a particular version of docker, then I don't necessarily want to change the version or source. In my case, this important not just for consistency & stability, but also for compliance reasons.

If docker is already present on the target machine, can we just use it? Trivial gate would be to do the install as:

provisioner.SSHCommand(fmt.Sprintf("sudo sh -c  'type /bin/docker || yum install -y --nogpgcheck  %s'", provisioner.DockerRPMPath))

ehazlett added 14 commits May 26, 2015 12:22
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
…cation

Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
@ehazlett ehazlett force-pushed the redhat-provisioning branch from 17efb95 to a8e2cd7 Compare May 26, 2015 16:22
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
ehazlett added a commit that referenced this pull request May 26, 2015
@ehazlett ehazlett merged commit 9bf7812 into docker:master May 26, 2015
@ehazlett ehazlett deleted the redhat-provisioning branch May 26, 2015 16:52
@denverdino
Copy link

@ehazlett Evan
After the 26432b7 the Native SSH client is broken. I got the following error when executing the following lines of "ssh/client.go"

    if err := session.RequestPty("xterm-256color", termHeight, termWidth, modes); err != nil {
        return string(output), err
    }

The error output for provisioning my ubuntu image is

SSH cmd err, output: EOF: 

And I think the logic of the Output method also has some problem for the command will be executed twice. Does it work as expected?

func (client NativeClient) Output(command string) (string, error) 
    ...
    output, err := session.CombinedOutput(command)
    ...
    return string(output), session.Run(command)
}

Thanks

@ehazlett
Copy link
Contributor Author

@sthulb do you know what is causing the Native to have issues?

@nathanleclaire is the Output func correct or did I munge a merge?

@nathanleclaire
Copy link
Contributor

Mmm, yeah that Output function definitely looks wrong. I'll take a look at it.

@ehazlett
Copy link
Contributor Author

ehazlett commented Jun 1, 2015

@nathanleclaire thx

}

// update OS -- this is needed for libdevicemapper and the docker install
if _, err := provisioner.SSHCommand("sudo yum -y update"); err != nil {

Choose a reason for hiding this comment

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

Is there a way to make this step be conditional/take a shorter amount of time? I already have a mirror on the local network, but this is taking up most of the setup time.

@ehazlett
Copy link
Contributor Author

Unfortunately this depends on the provider and mirror. If you have a local mirror then that's about all you can do. We must update as some images do not contain the proper device mapper libs that are needed.

@marcellodesales
Copy link

Hi @ehazlett, I'm trying to use the generic driver on RHEL 7.1... I have the following setup:

SSK keys already authorized on the host

~ ⌚ 1:04:08
$ ssh 10.132.52.146

Last login: Tue Jun 23 07:59:07 2015 from 172.17.193.49
[mdesales@pppdc9prd8ok ~]$ cat /etc/redhat-release 
Red Hat Enterprise Linux Server release 7.1 (Maipo)

Command to create host fails while importing keys

After a few seconds, the command to create and use the generic driver fails..

~ ⌚ 0:58:57
$ docker-machine create --driver "generic" --generic-ip-address "10.132.52.146" --generic-ssh-user "mdesales" dotci
Creating CA: /home/mdesales/.docker/machine/certs/ca.pem
Creating client certificate: /home/mdesales/.docker/machine/certs/cert.pem
Importing SSH key...
Error creating machine: exit status 1
You will want to check the provider to make sure the machine and associated resources were properly removed.

What should I do?

tomeon pushed a commit to tomeon/machine that referenced this pull request May 9, 2018
Refactor ntpd to support multiple servers
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.