Ha Guide to barclamp #2315

sjamgade · 2017-03-29T14:34:13Z

Basic extension

jgrassler

Thank you for this chapter! It contains most of what we need, but some parts are still a bit problematic (comments inline).

On a more general note, while links to the various components of crowbar-pacemaker are definitely a very good thing to have, do try to provide at least a few sentences of local explanation (especially for unintuitive stuff such as the lookups happening in CrowbarPacemakerHelper.haproxy_servers_for_service()). If there are only links the tabs are going to pile up in the reader's browser, making it hard to follow the text through all the back-and-forth between them.

jgrassler · 2017-03-29T15:21:06Z

doc/barclamp.md

+
+## High Availability (HA) Integration
+
+[This commit](https://github.com/crowbar/crowbar-openstack/pull/512/files) has


Careful, this was my first HA enabled barclamp and needed lots of fixing, so this pull request is far from perfect. Here are some of the follow-up commits:

crowbar/crowbar-openstack@9433bc5 (Wrong catalog entries for Barbican in the HA case)

crowbar/crowbar-openstack@cebcc2d (Fixes haproxy listening interface)

crowbar/crowbar-openstack@67a5e1f (Typos)
*https://github.com/crowbar/crowbar-openstack/commit/ fc5e44f1190663969549e7c16cdee76f55f77fee (use systemd service type rather than LSB service type pacemaker resource)

crowbar/crowbar-openstack@0729ec3 (sync marks missing from the original commit)

crowbar/crowbar-openstack@a082419 (in this case HA broke the non-HA scenario)

It's better to base this on current master and make the link permanent by linking to the current HEAD commit explicitly. You'll have to bend the prose around that of course and make it clear that the Barbican barclamp is the object of study, but it is better than referencing a faulty commit.

Maybe something like this: "in this section, we will use the Barbican barclamp as an example. You will find the barclamp's source code as we will describe and quote it throughout this here.

jgrassler · 2017-03-29T15:23:30Z

doc/barclamp.md

+
+-  [default.rb](https://github.com/crowbar/crowbar-openstack/pull/512/files#diff-f9a1e9d161032b6c0015336ab6812c14)
+
+   This file defines the default for all the values of the vars required by barclamp.


This could be a bit more verbose. This file allows you to set defaults (or overrides) for any attributes in the node object. In this case we'll use that capability to set a bunch of HA related attributes that nobody will have any business changing, hence we won't need go to the trouble of putting it into the data bag.

jgrassler · 2017-03-29T15:33:51Z

doc/barclamp.md

+
+- [barbican_service.rb(cookbook)](https://github.com/crowbar/crowbar-openstack/pull/512/files#diff-3f95710598349c27e4d1e79ba4cd7826)
+
+   Makes the installation of the service HA aware, by using [```provider  Chef::Provider::CrowbarPacemakerService```](https://github.com/crowbar/crowbar-ha/blob/master/chef/cookbooks/crowbar-pacemaker/providers/service.rb)


Please do not just link the commented code here. You can provide the link for reference, but try to explain that resource and why we need it rather than a normal service resource in the HA case.

I feel internal documentation if referenced is supposed to be read and not simply ctrl-clicked. But, yes some summary is a good heads-up. I will try to tar that file.

jgrassler · 2017-03-29T15:36:17Z

doc/barclamp.md

+
+- [api.rb](https://github.com/crowbar/crowbar-openstack/pull/512/files#diff-f9e37f7c8ba0f34831a43d2f89d87722)
+
+   We register the endpoint with keystone, using appropriate port.


Careful, we changed that later (it's broken in the original pull request).

jgrassler

This is only partial (only got as far as line 1418 so far), but there are already a few things that could do with adjusting. I'll try to have look at the rest on Monday.

jgrassler · 2017-03-31T14:58:28Z

doc/barclamp.md

+[This commit](https://github.com/crowbar/crowbar-openstack/pull/512/files) is
+the first place we start looking. It was not entirely correct and needed
+[more commits](https://github.com/crowbar/crowbar-openstack/commits/master/chef/cookbooks/barbican)
+to correct.


Maybe use this as a bit of an opportunity to encourage the reader ("HA is tricky so don't worry if your first attempt fails.") and point them at these commits more explicitly ("There are a lot of potential mistakes in getting HA working, maybe whatever you are struggling with right now turned up in the Barbican barclamp as well.")

jgrassler · 2017-03-31T14:59:45Z

doc/barclamp.md

+# HA attributes
+   [:enabled] = false
+
+# Ports to bind to when haproxy is used for the real ports


"Ports for the actual application to bind to" (less ambiguous).

jgrassler · 2017-03-31T15:00:41Z

doc/barclamp.md

+```
+> - Resource Agent: In 'LayMan' terms, resource agent is a script that the
+    cluster manager uses to manage the resource.
+> - LSB: is a specification of writing resource agent. 


Analogous to the systemd line below: "Use LSB init scripts to manage the resource"

jgrassler · 2017-03-31T15:17:37Z

doc/barclamp.md

+
+- [barbican_service.rb(cookbook)](https://github.com/crowbar/crowbar-openstack/commits/master/chef/cookbooks/barbican/definitions)
+
+   Makes the installation of the service HA aware, by using [```provider  Chef::Provider::CrowbarPacemakerService```](https://github.com/crowbar/crowbar-ha/blob/master/chef/cookbooks/crowbar-pacemaker/providers/service.rb)


This paragraph can probably be condensed a bit. Chef::Provider::CrowbarPacemakerService is essentially a dummy provider that does nothing and is used for the regular service resource in the HA case: this ensures crowbar does enable/start the service because that is up to the Pacemaker resource agent in this case.

Also, the Barbican barclamp in its current shape does not use it, because it only runs services that are horizontally scalable anyway (such as barbican-worker). There are some vestiges in definitions/barbican.rb, but nothing uses that anymore.

You'll find CrowbarPacemakerService in use in the Neutron barclamp, for example: https://github.com/crowbar/crowbar-openstack/blob/f26dc78fe3038ebb5bdb426e70d6750f2351cf29/chef/cookbooks/neutron/recipes/server.rb#L374

Also you only need single backticks here. Triple backticks only make sense for multi-line blocks, or blocks which might contain literal backticks.

jgrassler · 2017-03-31T15:18:56Z

doc/barclamp.md

+use **service resource** from that provider and not the default one. The file has huge documentation
+and is good read for some internal interactions between chef and pacemaker. Chef tries to manage
+resources locally, while pacemaker operations are cluster-wide. So we need special ways
+to interact with pacemaker about status of clone/group/service and its configuration.


Please try to explain "clone/group/service" - the reader might not have the background to know what these refer to.

jgrassler · 2017-03-31T15:59:34Z

doc/barclamp.md

+
+- [api.rb](https://github.com/crowbar/crowbar-openstack/commits/master/chef/cookbooks/barbican/recipes/api.rb)
+
+   First make sure the service and user are correctly registered witht keystone, use keystone_setting_helper to get keystone endpoint details. Then register the endpoint with keystone, using appropriate port.


Please explain what this file does (it deploys the barbican-api service itself) and explain the changes you need to make to it to accommodate ha.rb in the HA case, i.e. the following among others:

Create sync marks to ensure the Keystone resources exist when the service starts on any node

Create sync marks to ensure the database exist (in Barbican's case it's in common.rb but sometimes it is created alongside the service itself).

Ensure the listening address is retrieved using CrowbarHelper.get_host_for_public_url()

Ensure the recipe configures the correct port to bind to depending on whether HA is enabled or not

aspiers

Thanks a lot for working on this! It will be a really useful addition to the existing guide.

It looks like Johannes has already given you a lot of great feedback on the content. Just to keep things varied, I instead chose to be the annoying pedant about Markdown syntax and language spelling / grammar ;-) Sorry about that. But I hope you'll agree that it's worth spending time on writing high quality developer docs. The team is growing fast, so we'll really benefit from that.

Oh, please can you also squash the commits into a single one and make the commit message adhere to our conventions? You may find my blog post and this OpenStack guide useful if you are not already familiar with this kind of thing.

Thanks a lot and keep up the good work!

aspiers · 2017-04-02T23:59:42Z

doc/barclamp.md

+
+## High Availability (HA) Integration
+
+Using the [barbican barclamp](https://github.com/crowbar/crowbar-openstack/tree/9805c7ddb81ffbf6f2b79061c6ceb332659cd614/chef/cookbooks/barbican)


Did you deliberately link to a specific commit here? Might make more sense to link to master.

Well the idea was, to navigate the user through the changes applied on a barclamp to make it HA compatible. (in continuation to the existing idea of creating a new barclamp)
But I guess it was not really reflected. If you still think I can correct that link.

Will linking the history of commits to that folders be useful ?
https://github.com/crowbar/crowbar-openstack/commits/master/chef/cookbooks/barbican

aspiers · 2017-04-03T00:00:20Z

doc/barclamp.md

+## High Availability (HA) Integration
+
+Using the [barbican barclamp](https://github.com/crowbar/crowbar-openstack/tree/9805c7ddb81ffbf6f2b79061c6ceb332659cd614/chef/cookbooks/barbican)
+as an example. We will try to investigate what changes are required to make a barclamp


"as an example, we will ..."

aspiers · 2017-04-03T00:00:48Z

doc/barclamp.md

+
+Using the [barbican barclamp](https://github.com/crowbar/crowbar-openstack/tree/9805c7ddb81ffbf6f2b79061c6ceb332659cd614/chef/cookbooks/barbican)
+as an example. We will try to investigate what changes are required to make a barclamp
+HA compatible


Missing trailing full stop.

No missing anymore

aspiers · 2017-04-03T00:01:32Z

doc/barclamp.md

+
+-  [default.rb](https://github.com/crowbar/crowbar-openstack/commits/master/chef/cookbooks/barbican/attributes/default.rb)
+
+   This file defines the default for all the values of the vars required by barclamp.


Don't indent the beginning of normal text paragraphs, since indentation has specific meaning in Markdown.

I wanted a different reading style

Switched to what you suggested :-)

aspiers · 2017-04-03T00:01:50Z

doc/barclamp.md

+
+   This file defines the default for all the values of the vars required by barclamp.
+It allows you to set defaults (or overrides) for any attributes in the node object.
+In this case we will use that capability to set a bunch of HA related attributes that


"HA-related"

aspiers · 2017-04-03T00:05:53Z

doc/barclamp.md

+
+- [helpers.rb](https://github.com/crowbar/crowbar-openstack/commits/master/chef/cookbooks/barbican/libraries/)
+
+    Here we define a helper class to initialize the variables/ports correctly ```if ha_enabled```


Only single backticks needed here.

aspiers · 2017-04-03T00:06:18Z

doc/barclamp.md

+
+- [api.rb](https://github.com/crowbar/crowbar-openstack/commits/master/chef/cookbooks/barbican/recipes/api.rb)
+
+   First make sure the service and user are correctly registered witht keystone, use keystone_setting_helper to get keystone endpoint details. Then register the endpoint with keystone, using appropriate port.


Also drop the initial indent, and there's a witht typo.

aspiers · 2017-04-03T00:06:30Z

doc/barclamp.md

+   sent to VIP in case of HA.(which is actually the cluster address where HAproxy is
+   listening)
+
+


Too many blank lines here.

aspiers · 2017-04-03T00:07:13Z

doc/barclamp.md

+	```
+    If using ha, host address will be from admin network, as HAProxy will listen
+    virtual-ip-address form admin network, as the node is part of the cluster.
+    Otherwise just register on '*'


Should be

on `*`

aspiers · 2017-04-03T00:08:36Z

doc/barclamp.md

+
+
+
+> To make sure the other nodes do not start accessing the database before the


I think you probably don't want the leading > here, as that has a specific meaning in GH-flavoured markdown.

sjamgade · 2017-04-03T08:36:24Z

I am using this. Please do let me know if this is not a good enough tool.

And will using a url shortner be a good idea. Mind if I suggest this https://www.git.io/

aspiers · 2017-04-04T10:02:39Z

@sjamgade I think it's OK to use a Markdown editor, but you still need to know how to write Markdown which is both correct and considered good style. Hopefully my comments should have helped with that already.

In what context do you want to use a URL shortener, and why? The downside is that the extra level of indirection obfuscates the navigation, and I don't know what the upsides would be.

aspiers

Thanks, getting better already. Still some nitpicks from my side (sorry!)

aspiers · 2017-04-04T10:03:55Z

doc/barclamp.md

@@ -1356,3 +1356,154 @@ place) you will have to add additional setup code to
 and/or
 [qa_crowbarsetup.sh](https://github.com/SUSE-Cloud/automation/blob/master/scripts/qa_crobarsetup.sh)


I just noticed a crobar typo here. Not related to this PR but maybe you could fix anyway.

Thanks a lot for that. Corrected.

aspiers · 2017-04-04T10:05:16Z

doc/barclamp.md

+## High Availability (HA) Integration
+
+Using the [barbican barclamp](https://github.com/crowbar/crowbar-openstack/tree/master/chef/cookbooks/barbican)
+as an example. We will try to investigate what changes are required to make a barclamp


"as an example, we will ..."

aspiers · 2017-04-04T10:08:24Z

doc/barclamp.md

+as an example. We will try to investigate what changes are required to make a barclamp
+HA compatible.
+
+[This commit](https://github.com/crowbar/crowbar-openstack/pull/512/files) is


"This commit" is not the best text label for this hyperlink. Firstly, the URL links to a PR not a commit, so it should be "This PR". But that still doesn't tell the reader anything about what the PR is, until they follow the link. Please read this W3C article explaining how to write good links.

Re-Written !

aspiers · 2017-04-04T10:27:55Z

doc/barclamp.md

+
+-  [default.rb](https://github.com/crowbar/crowbar-openstack/commits/master/chef/cookbooks/barbican/attributes/default.rb)
+
+   This file defines the defaults for all the values of the vars required by barclamp.


I wanted a different reading style

Switched to what you suggested :-)

Oh, now I see what you were trying to do. Yes, it makes sense to structure things this way, but there are two style issues:

You should use 4 column indents as dictated by https://daringfireball.net/projects/markdown/syntax#list

The whole paragraph should be indented, not just the first line.

I've created a more in-depth explanation of why these style principles are important.

thanks a lot for writing that. Explains a lot. Stackexhange also had something interesting about the number of spaces to get indented code blocks

Done!

aspiers · 2017-04-04T10:28:56Z

doc/barclamp.md

+   This file defines the defaults for all the values of the vars required by barclamp.
+It allows you to set defaults (or overrides) for any attributes in the node object.
+In this case we will use that capability to set a bunch of HA-related attributes that
+are rarely changed and, hence need not be put int the data bag.


You only corrected one of the two typos I pointed out.

aspiers · 2017-04-04T10:33:31Z

doc/barclamp.md

+- [`helpers.rb`](https://github.com/crowbar/crowbar-openstack/commits/master/chef/cookbooks/barbican/libraries/)
+
+    Here we define a helper class to initialize the variables/ports correctly `if ha_enabled`
+


Only blank one line between list items.

aspiers · 2017-04-04T10:34:51Z

doc/barclamp.md

+operations are cluster-wide. So we need special ways to interact with pacemaker
+about status of [clone/group/service](http://clusterlabs.org/doc/en-US/Pacemaker/1.1/html/Pacemaker_Explained/ch10.html)
+and its configuration. As the link has good starting explanation about
+clone/group and would help understand the need of a custom `service resource`


Backticks should only be used on code and machine-readable / machine-writable text. I think double-quotes are more appropriate for "service resource", although actually no quotes are needed.

aspiers · 2017-04-04T10:35:53Z

doc/barclamp.md

+clone/group and would help understand the need of a custom `service resource`
+Also, the Barbican barclamp in its current shape does not use it, because it
+only runs services that are horizontally scalable anyway (such as
+barbican-worker). There are some vestiges in definitions/barbican.rb, but


barbican-worker and definitions/barbican.rb are both machine-readable so deserve backticks.

"looks better now"

aspiers · 2017-04-04T10:37:03Z

doc/barclamp.md

+
+- [`common.rb`](https://github.com/crowbar/crowbar-openstack/commits/fb4cd02a09ba35182e401cac48858e2e5d1d5508/chef/cookbooks/barbican/recipes/common.rb)
+
+   Common recipe is executed for all services and on all nodes of a cluster in


Please make sure your indentation is consistent. This paragraph is indented 3 columns, which I think should be pretty much always avoided in Markdown. As explained before, 4 columns is best, but 2 is OK in limited circumstances.

Yes, Corrected.

aspiers · 2017-04-04T10:37:19Z

doc/barclamp.md

+   sent to VIP in case of HA.(which is actually the cluster address where HAproxy is
+   listening)
+
+> To make sure the other nodes do not start accessing the database before the


Ditch the blockquote > here too.

sjamgade · 2017-04-05T09:35:20Z

@aspiers Url shortner so that eyes dont hurt when reading the un-rendered text.
But I guess showing the full url helps in learning the folder structure.

Thanks a lot for your guidance.

matrixik · 2017-04-07T08:19:52Z

reposync_crowbar should be updated to work with two different repositories.

jgrassler · 2017-04-07T13:14:31Z

reposync_crowbar should be updated to work with two different repositories.

That's not really the focus of this pull request, I'm afraid :-(

We could create a second pull request for that, but I don't think it's a good idea: while I know that it would come in handy for the additional crowbar-ha diff you currently need to apply, I'd prefer not to include this feature in reposync_crowbar. In fact I'd rather avoid including any additional features in reposync_crowbar. I included it in the guide as a teaching aid for describing all the steps involved in turning a pull request diff into code running in an active Crowbar instance. It was never meant to be complete and full-featured; quite the opposite actually: I wanted it to be as short as possible (or only as long as needed, if you prefer) because lots of people will hack up their own tooling anyway because they happen not to like the reposync_crowbar approach. Every feature I add to it will make it harder for people to use it for its original purpose: reading up on how to get a barclamp diff into a Crowbar instance.

On that note: how about copying that bashrc into out into a separate script that takes the repository to use as a parameter? It wouldn't be a big change (you could well do it locally) and it will do the job you need it to do for now.

aspiers · 2017-04-10T10:50:10Z

because lots of people will hack up their own tooling anyway because they happen not to like the reposync_crowbar approach

And that's a fundamental problem - NIH Syndrome will probably continue to plague us for ever :-(

sjamgade added do not merge yet wip labels Mar 29, 2017

sjamgade self-assigned this Mar 29, 2017

dirkmueller requested review from aspiers and jgrassler March 29, 2017 14:45

jgrassler reviewed Mar 29, 2017

View reviewed changes

sjamgade force-pushed the haguide branch from d1511e9 to 7f5ba7d Compare March 30, 2017 16:00

jgrassler requested changes Mar 31, 2017

View reviewed changes

aspiers requested changes Apr 3, 2017

View reviewed changes

sjamgade force-pushed the haguide branch from 7f5ba7d to 8cccaca Compare April 3, 2017 08:32

aspiers requested changes Apr 4, 2017

View reviewed changes

sjamgade force-pushed the haguide branch from 8cccaca to 946d5e4 Compare April 5, 2017 09:28

WIP: barclamp guide (HA)

c45ded6

sjamgade force-pushed the haguide branch from 946d5e4 to c45ded6 Compare April 11, 2017 08:47

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


		## High Availability (HA) Integration

		[This commit](https://github.com/crowbar/crowbar-openstack/pull/512/files) has


		- [default.rb](https://github.com/crowbar/crowbar-openstack/pull/512/files#diff-f9a1e9d161032b6c0015336ab6812c14)

		This file defines the default for all the values of the vars required by barclamp.


		- [barbican_service.rb(cookbook)](https://github.com/crowbar/crowbar-openstack/pull/512/files#diff-3f95710598349c27e4d1e79ba4cd7826)

		Makes the installation of the service HA aware, by using [```provider Chef::Provider::CrowbarPacemakerService```](https://github.com/crowbar/crowbar-ha/blob/master/chef/cookbooks/crowbar-pacemaker/providers/service.rb)


		- [api.rb](https://github.com/crowbar/crowbar-openstack/pull/512/files#diff-f9e37f7c8ba0f34831a43d2f89d87722)

		We register the endpoint with keystone, using appropriate port.


		- [api.rb](https://github.com/crowbar/crowbar-openstack/commits/master/chef/cookbooks/barbican/recipes/api.rb)

		First make sure the service and user are correctly registered witht keystone, use keystone_setting_helper to get keystone endpoint details. Then register the endpoint with keystone, using appropriate port.


		## High Availability (HA) Integration

		Using the [barbican barclamp](https://github.com/crowbar/crowbar-openstack/tree/9805c7ddb81ffbf6f2b79061c6ceb332659cd614/chef/cookbooks/barbican)


		- [helpers.rb](https://github.com/crowbar/crowbar-openstack/commits/master/chef/cookbooks/barbican/libraries/)

		Here we define a helper class to initialize the variables/ports correctly ```if ha_enabled```

		sent to VIP in case of HA.(which is actually the cluster address where HAproxy is
		listening)




		> To make sure the other nodes do not start accessing the database before the

		@@ -1356,3 +1356,154 @@ place) you will have to add additional setup code to
		and/or
		[qa_crowbarsetup.sh](https://github.com/SUSE-Cloud/automation/blob/master/scripts/qa_crobarsetup.sh)


		- [default.rb](https://github.com/crowbar/crowbar-openstack/commits/master/chef/cookbooks/barbican/attributes/default.rb)

		This file defines the defaults for all the values of the vars required by barclamp.

		- [`helpers.rb`](https://github.com/crowbar/crowbar-openstack/commits/master/chef/cookbooks/barbican/libraries/)

		Here we define a helper class to initialize the variables/ports correctly `if ha_enabled`


		- [`common.rb`](https://github.com/crowbar/crowbar-openstack/commits/fb4cd02a09ba35182e401cac48858e2e5d1d5508/chef/cookbooks/barbican/recipes/common.rb)

		Common recipe is executed for all services and on all nodes of a cluster in

Ha Guide to barclamp #2315

Are you sure you want to change the base?

Ha Guide to barclamp #2315

Conversation

sjamgade commented Mar 29, 2017

jgrassler left a 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

jgrassler left a 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

aspiers left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sjamgade Apr 3, 2017 • edited

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

sjamgade Apr 3, 2017 • edited

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

sjamgade commented Apr 3, 2017

aspiers commented Apr 4, 2017

aspiers left a 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

sjamgade commented Apr 5, 2017

matrixik commented Apr 7, 2017

jgrassler commented Apr 7, 2017 • edited

aspiers commented Apr 10, 2017

sjamgade Apr 3, 2017 •

edited

sjamgade Apr 3, 2017 •

edited

jgrassler commented Apr 7, 2017 •

edited