Initial Push of Azure Ansible Ref Arch Tempaltes by glennswest · Pull Request #125 · openshift/openshift-ansible-contrib

glennswest · 2016-11-22T01:52:44Z

Significant Changes:

Change ose_pv_create.sh to be transparently encoded in store.sh
Removed Centos Image selection - Origin script did not support centos
Fixed external reference to ssh config and made it internal
Fixed spelling and branding issues
Created automated script to fix pathing issues - Should reduce pathing issues on pull
Added additional attribution.

Architecture

Each VM type has qty 1 ARM (Azure Resource Manager) Template and qty 1 bash script,
all other scripts, playbooks etc are created within the 1 script. Part of this is based on the way
Azure Linux Extensions work, and the rest is done for transparency.

openshift-bot · 2016-12-02T15:53:03Z

Can one of the admins verify this patch?
I understand the following comments:

bot, add author to whitelist
bot, test pull request
bot, test pull request once

openshift-bot · 2016-12-03T23:28:04Z

Can one of the admins verify this patch?

openshift-bot · 2016-12-03T23:43:02Z

Can one of the admins verify this patch?
I understand the following comments:

bot, add author to whitelist
bot, test pull request
bot, test pull request once

cooktheryan · 2016-12-05T21:35:52Z

reference-architecture/azure-ansible/testcases/test_pv.sh

@@ -0,0 +1,8 @@
+oc new-project demo


Do you want to try to migrate to the common validation playbooks here? Less work to maintain and closer to the ansible migration

not at this point, as I want to tech freeze and get doc finished. Then can add lots of value I have already got hooks for 3.5 release.

cooktheryan · 2016-12-07T03:17:17Z

reference-architecture/azure-ansible/azureexpand.json

+    "rhel" : {
+      "publisher" : "RedHat",
+      "offer" : "RHEL",
+      "sku" : "7.2",


Should we ship 7.3 due to the requirements for OpenShift 3.4

If worried about ocp 3.3 on rhel we shipped it about a month ago on aws with no issues

Not there yet. This is currently 3.3. So when 3.4 comes, so will only change at becomes available. So not changable till then.

ok, 7.3 is available will change.

cooktheryan · 2016-12-07T03:19:24Z

reference-architecture/azure-ansible/bastion.json

+          "fileUris" : [
+            "[concat(parameters('baseTemplateUrl'), 'bastion.sh')]"
+          ],
+          "commandToExecute" : "[ concat('sh bastion.sh ', resourceGroup().name, ' ', parameters('adminUsername'), ' ', parameters('adminPassword'), ' ', reference(concat(parameters('vmName'), 'pip')).dnsSettings.fqdn, ' ', padLeft(parameters('numberOfNodes'), 2, '0'), ' ', parameters('routerExtIP'),' ', parameters('RHNUserName'), ' ', parameters('RHNPassword'),' ', parameters('SubscriptionPoolId'), ' ',  variables('sQuote'),  parameters('sshPrivateData'), variables('sQuote'),  ' ', variables('sQuote'), parameters('sshKeyData'), variables('sQuote')  ) ]"


Probably just a small NIT but maybe change the var to RHSM rather than RHN. It should make retro fitting to common plays easier.

this is actually a variable between ARM template and the bash template. Has no material effect.

You are correct that the prefixing your credential variables with RHN has no material effect in this case, however, the reasoning behind @cooktheryan's Agile based suggestion of standardizing the credential variables by using the prefix RHSM over RHN, will go a long way in insuring that future refactoring efforts will be less painful. I'm certain that @cooktheryan wasn't trying to single you out in this comment, the avoidance of creating technical debt right from the start is very relevant and needs all of our attention. Standardizing these common variables is something we, as a team, all need to work together on. We could tack this on to #117 or create a new issue, but either way, doing the work up front will pay dividends in the future. /cc @dav1x @markllama @scollier

+1 since the vmware and gce plays all used the same var names it has made #126 that much easier. The common vars also allowed for me to use all the same plays regardless of provider which is our end goal in getting this work productized.

cooktheryan · 2016-12-07T03:21:27Z

reference-architecture/azure-ansible/bastion.json

+            "properties" : {
+              "protocol" : "Tcp",
+              "sourcePortRange" : "*",
+              "destinationPortRange" : "9090",


Will you be proxying access to the cockpit UI through the bastion?

cooktheryan · 2016-12-07T03:23:07Z

reference-architecture/azure-ansible/bastion.sh

+chmod 600 /root/.ssh/id_rsa
+
+sleep 30
+cat <<EOF > /root/setup_ssmtp.sh


Have you tested failing the mail step? The reason why I ask is that it would be a bummer if the installation hung just due to an email. Also, is the email portion required or optional? Could it be configured to be optional if it is currently required?

Yes. Many times. It runs fine without the mail even being there. (In early I faced that quite a few times, so its safe)

cooktheryan · 2016-12-07T03:27:25Z

reference-architecture/azure-ansible/bastion.sh

+
+cat <<EOF > /etc/sysconfig/docker-storage-setup
+DEVS=/dev/sdc
+VG=docker-vg


Might be worth adding this in as well
DATA_SIZE=95%VG
EXTRA_DOCKER_STORAGE_OPTIONS="--storage-opt dm.basesize=3G"

The extra storage options will help with quota and I can't remember right. Also, Im not sure how much space of the vg that is created is used. It might be worth testing or providing a value.

Will remove docker support from bastion.

cooktheryan · 2016-12-07T03:28:04Z

reference-architecture/azure-ansible/bastion.sh

+subscription-manager repos     --enable="rhel-7-server-ose-3.3-rpms"
+yum -y install atomic-openshift-utils
+yum -y install git net-tools bind-utils iptables-services bridge-utils bash-completion httpd-tools
+yum -y install docker


Will you need docker on the bastion?

There is support there for it, but at the moment no.

will remove docker support from bastion.

cooktheryan · 2016-12-07T03:29:20Z

reference-architecture/azure-ansible/bastion.sh

+rhn_pool_id=${RHNPOOLID}
+openshift_install_examples=true
+deployment_type=openshift-enterprise
+openshift_master_identity_providers=[{'name': 'htpasswd_auth', 'login': 'true', 'challenge': 'true', 'kind': 'HTPasswdPasswordIdentityProvider', 'filename': '/etc/origin/master/htpasswd'}]


I wouldn't do HTPasswd esp if we are multi-master. You should be able to borrow gmail, github, or ldap creds from one of the completed archs

Actually plan is HTPASSWD is "emergency login", Ive done the work for keycloak already, and will be the auth solution. This has been the strategy since day one. With keycloak everything can be integrated. I've done initial testing with it, Im only missing the auto setup of the providers.

cooktheryan · 2016-12-07T03:29:51Z

reference-architecture/azure-ansible/bastion.sh

+remote_user=${AUSERNAME}
+
+openshift_master_default_subdomain=${ROUTEREXTIP}.xip.io 
+openshift_use_dnsmasq=False


@detiber I think you had a comment about this before but i can't remember if there was a resolution

I did test this, and everything appears to be working. Did read thru the docs, and far as I can tell, everything is functional. So its technical risk at this port to change it.

cooktheryan · 2016-12-07T03:30:14Z

reference-architecture/azure-ansible/bastion.sh

+openshift_public_hostname=${RESOURCEGROUP}.trafficmanager.net
+
+openshift_master_cluster_method=native
+openshift_master_cluster_hostname=${RESOURCEGROUP}.trafficmanager.net


are these hard coded domains correct?

Yes, and working. And mandated via azure. Supports multiple clusters, driven by resource group.

Is there a play that sets the domain for a customers domain?

First, we do give them a change to specify one within the Azure naming conventions. Then its full auto. If they want a totally separate DNS server or domain outside, they can manually add it.

cooktheryan · 2016-12-07T03:34:12Z

reference-architecture/azure-ansible/bastion.sh

+# default selectors for router and registry services
+openshift_router_selector='region=infra'
+openshift_registry_selector='region=infra'
+


You should be able to scale router and registry here based off of the amount of infra nodes you have

Can consider in the future, as I need to look has to get this to happen.

cooktheryan · 2016-12-07T03:34:32Z

reference-architecture/azure-ansible/bastion.sh

+# default selectors for router and registry services
+openshift_router_selector='region=infra'
+openshift_registry_selector='region=infra'
+


Should probably provide a default node selector

cooktheryan · 2016-12-07T03:37:33Z

reference-architecture/azure-ansible/bastion.sh

+master2 openshift_node_labels="{'region':'master','zone':'default'}" 
+master3 openshift_node_labels="{'region':'master','zone':'default'}" 
+node[01:${NODECOUNT}] openshift_node_labels="{'region': 'primary', 'zone': 'default'}"
+infranode1 openshift_node_labels="{'region': 'infra', 'zone': 'default'}"


Im not sure if region is the best naming for the labels. I think once we move to multi-region or more federated deployments region may be used to show the location of the nodes geographically. Most of the providers used role rather than region. As for the node label rather than primary we used app. Completely optional on the primary vs app but keeps the arks rather in sync.

can change the nodes to "app" if you think thats sensible. The above is based on documentation, so which is better, stick to doc, or copy other providers?

cooktheryan · 2016-12-07T03:39:28Z

reference-architecture/azure-ansible/bastion.sh

+cat <<EOF > /home/${AUSERNAME}/subscribe.yml
+---
+- hosts: all
+  vars:


I believe in the previous PR @detiber had suggested serial during the registration process. If not. This should probably be in some sort of serial format just to prevent a herd trying to register all at once.

actually the subscription with retry that I've implemented is solid, with large number of tests, that no matter what I'd get failures till I did the retry logic. With the retry logic its very solid and works every time. So going serial and without the retries would signficantly increase failures and "bad" days where it just didnts work.

Also there are issues in subscribe ansible module that are new in ansible 2.1

Glenn, can you point to or create a BZ for this issue. There are hundreds of people who rely on the official "Subscription Manager" module, so if you have a reproducible bug, make sure to at least report it or even better, submit a patch.

cooktheryan · 2016-12-07T03:41:18Z

reference-architecture/azure-ansible/bastion.sh

+    ignore_errors: yes
+  - name: disable all repos
+    shell: subscription-manager repos --disable="*" 
+  - name: enable rhel7 repo


Rather than using shell for all of the repos I would suggest using the already used playbooks/roles/rhsm-repos/tasks/main.yaml

this was changed due to ansible problems.

cooktheryan · 2016-12-07T03:41:57Z

reference-architecture/azure-ansible/bastion.sh

+  - name: install the latest version of PyYAML
+    yum: name=PyYAML state=latest
+  - name: Install the ose client
+    yum: name=atomic-openshift-clients state=latest


Probably not required as this should be taken care of during the install on all hosts except the bastion

found it useful for checkout and debug, as it was not happening.

cooktheryan · 2016-12-07T03:42:30Z

reference-architecture/azure-ansible/bastion.sh

+    async: 1200
+    poll: 10
+  - name: Wait for Things to Settle
+    pause:  minutes=5


I would potentially add in NetworkManager installation and the starting and enabling

It is installed in all the hosts except bastion.
So is there a requirement on the bastion host?

no it will not be required

cooktheryan · 2016-12-07T03:43:58Z

reference-architecture/azure-ansible/bastion.sh

+  - name: Change Node perFSGroup Qouta Setting
+    lineinfile: dest=/etc/origin/noce/node-config.yaml regexp=^perFSGroup: line="    perFSGroup:512Mi"
+  - name: Update Mount to Handle Quota
+    mount: fstype=xfs name=/var/lib/origin/openshift.local/volumes src=/dev/sdd option="gquota" state="mounted"


Is it possible to cloud init this task?

Actually potentially could move this into per node scripts, but ansible does seem to do it fine.

cooktheryan · 2016-12-07T03:44:50Z

reference-architecture/azure-ansible/bastion.sh

+  - name: Create Master Directory
+    file: path=/etc/origin/master state=directory
+  - name: add initial user to OpenShift Enterprise
+    shell: htpasswd -c -b /etc/origin/master/htpasswd ${AUSERNAME} ${PASSWORD}


Definitely look towards another auth

cooktheryan · 2016-12-07T03:48:18Z

reference-architecture/azure-ansible/infranode.json

+              "priority" : 2000,
+              "direction" : "Inbound"
+            }
+          }


This may be answered later on but is 22 allowed only from the bastion or 22 allowed from both internal and external to the infra nodes?

22 only from bastion host.

cooktheryan · 2016-12-07T03:49:13Z

reference-architecture/azure-ansible/node.sh

+#!/bin/bash
+
+#yum -y update
+yum -y install wget git net-tools bind-utils iptables-services bridge-utils bash-completion docker


I dont think you need these packages as the installer should take care of docker and possibly any other items.

THis is recommended from the doc. https://docs.openshift.com/container-platform/3.3/install_config/install/host_preparation.html

So how do u want to handle?

I believe these packages are only in required for the host performing the installation. If you check out the aws plays we don't actually install those packages.

cooktheryan · 2016-12-07T03:52:02Z

reference-architecture/azure-ansible/node.sh

+EXTRA_DOCKER_STORAGE_OPTIONS="--storage-opt dm.basesize=3G"
+EOF
+
+docker-storage-setup                                                                                                                                    


Id let the installer handle storage setup and the enabling of docker. This may be a good location though to do you mounts

This is from the documentation, plus the 3G was recommended in a prevous commit.
How to handle the difference?

Maybe it was a bad comment line. I would just remove the step of actually running docker-storage-setup yourself

cooktheryan · 2016-12-07T03:54:31Z

reference-architecture/azure-ansible/logging.sh

+chmod 600 /root/.ssh/id_rsa
+
+subscription-manager unregister 
+subscription-manager register --username $RHNUSERNAME --password $RHNPASSWORD


Could you just re-use the existing ansible items from above here?

Note the intention of doing it here, was to get the logging host up so it could provide logging before everything is ready. For the moment I'm deleting logging host.

cooktheryan · 2016-12-07T03:55:13Z

reference-architecture/azure-ansible/master.json

+              "protocol" : "Tcp",
+              "sourcePortRange" : "*",
+              "destinationPortRange" : "22",
+              "sourceAddressPrefix" : "*",


I feel like master 22 access should be limited from the the bastion only

Funny, it should be the same as infra, will dig and figure out why.

cooktheryan · 2016-12-07T03:56:12Z

reference-architecture/azure-ansible/master.json

+            "properties" : {
+              "protocol" : "Tcp",
+              "sourcePortRange" : "*",
+              "destinationPortRange" : "8443",


Im not sure how azure handles vips but is it possible to use 443, the console_port variable at install time, and set the vip to use 443 as well? Just to use standard ports I remember how relieved i was to move from 8443 to 443

@glennswest will this be possible to change?

I won't block on this, but please consider standardizing the port for the same reasons as stated above #125 (comment)

…nd script names that are external cannot be changed

…varaible names and functional areas

…and names

…e is correct intent.

…ontainer Platform

glennswest · 2016-12-14T08:29:28Z

At this point, I've processed the list of suggestions, with the following exceptions:

Follow Azure Practice of one customization script per node type - ie embedded scripts.
I've left the way the ssmtp package is pulled, as its RedHat best practice to allow updates, the suggested change would hard-code a single version, which is not required technically.
Fix some png images that may be done in a better way. (Windows ssh generation).

On the commit on auth: It will be done shortly in keycloak, but is out of scope for this commit.

themurph · 2016-12-15T00:47:40Z

@glennswest Great job on refactoring, resolving, answering, and pushing out all these requests for changes. Approving and merging.

Initial Push of Azure Ansible Ref Arch Tempaltes

Cleaned up and made more audible

7dadee0

cooktheryan mentioned this pull request Nov 22, 2016

Azure PR #115

Closed

cooktheryan reviewed Dec 5, 2016

View reviewed changes

cooktheryan reviewed Dec 7, 2016

View reviewed changes

glennswest added 25 commits December 13, 2016 15:32

Remove onevm functionality

1aa8b44

Remove onenode functionality

1b7b9da

Remove azureexpand functionality

5fa1cb7

Change Redhat to Red Hat except for publisher, which required by azure

375397b

Change textual references to ose to OCP - repo names, ansible vars, a…

e67a3fe

…nd script names that are external cannot be changed

Change OS X to macOS

b7d767a

Change Centos to CentOS

552b4ba

Remove addfunction as onevm etc have been removed

40c44da

Change Openshift to Openshift in descriptive elements, do not change …

6e4f4e6

…varaible names and functional areas

Remove rui mispelling, and Remove closed bug

9235f6d

Change rsa to RSA for descriptive references - Not for script or comm…

603beb2

…and names

Add securestring to sshprivatedata

787d33f

Fix comment on ssmtp script

10cde61

Make sure rhui is RHUI

e74b7c4

Fix capitalization of load balancer

0d6eb6e

Fix descriptive iscsi references to iSCSI

c9808c8

Change comment on restarting iscsid to iSCSI. Restart of iscsi servic…

d3a4b05

…e is correct intent.

Fix double space in systemctl start iscsi comment

0becab5

Fix ansible task description for starting iSCSI discovery on all hosts

93a7348

Fix comment on ocp product name - Openshift Enterprise to OpenShift C…

d775e11

…ontainer Platform

Removed commented out lines

62e7490

Fix domain for app spelling error

b8befc0

Fix dns to DNS in descriptive text

0dc2a4f

FIx heading for SSH Key Generation On Windows

0aa2887

Remove host name

fea0c63

themurph approved these changes Dec 15, 2016

View reviewed changes

themurph merged commit 122f75b into openshift:master Dec 15, 2016

jaywryan pushed a commit to jaywryan/openshift-ansible-contrib that referenced this pull request Jul 3, 2018

Merge pull request openshift#125 from glennswest/master

beffc20

Initial Push of Azure Ansible Ref Arch Tempaltes

Conversation

glennswest commented Nov 22, 2016

Uh oh!

openshift-bot commented Dec 2, 2016

Uh oh!

openshift-bot commented Dec 3, 2016

Uh oh!

openshift-bot commented Dec 3, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

themurph Dec 14, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

themurph Dec 14, 2016 •

edited

Loading