OpenStack Cinder Initial Commit by intlabs · Pull Request #78 · att-comdev/openstack-helm

intlabs · 2017-01-02T02:20:25Z

OpenStack Cinder

This commit introduces Cinder.

This change is

alanmeadows · 2017-01-03T23:57:08Z

@@ -0,0 +1,73 @@
+{{- define "joinListWithColon" -}}


This can now be removed it is officially in common, incorrect name and all.

alanmeadows · 2017-01-04T00:07:30Z

+{{ range $k, $v := . }}{{ if $k }},{{ end }}{{ $v }}{{ end }}
+{{- end -}}
+
+{{- define "env_admin_openrc" }}


I think we can put this whole thing in common at this point and give it a go as the method for endpoint and service registration.

We'll update the other charts as required when we feel its reached its completion. Because it pulls from values from the running chart, could not the entire definition "clone-able" init container block with input just for the unique service name? All of the data it needs, such as the service endpoints themselves would come from endpoints: in values and I see no reason we could not take the same approach for common keystone authentication to avoid secret sprawl. The per service secrets concerns me if I am reading this right, because would cinder ever need unique admin credentials? It only needs (potentially) unique service credentials, again which could come from values if we adopt a common keystone: layout in each chart.

In other words, if all charts have a common endpoints: definition and a common keystone: pattern, wouldn't you have enough to simply include a complete init-container/job/whatever in one compact line in any chart and tell it which part of the endpoints: and keystone: trees its pulling from this time....

alanmeadows · 2017-01-04T00:09:52Z

@@ -0,0 +1,7 @@
+apiVersion: v1


As mentioned in the heat PR for now we're going with configmap-etc and configmap-bin - it does have the annotation drawback you referenced as potentially triggering a "global" restart but we can address that separately as we refine how "etc" or "bin" changes should effect the running system. Annotations may not be the right way in the end but we need to show we at least have a path.

alanmeadows · 2017-01-04T00:14:09Z

+enable_v1_api = false
+volume_name_template = %s
+
+osapi_volume_workers = {{ .Values.api.workers }}


As an aside, I've been "loosely" following in this specific example something like .Values.cinder.api.default.osapi_volume_workers which maps fairly closely to the raw config file path because over time I imagine we will convert every toggle to a values parameter the end user can control, and without some pattern it will be difficult to know what to twiddle. With only a few knobs to turn, your .Values layout is supreme, but I think we will find it falls apart with large numbers of tunables of which no chart has yet because each developer will think differently about how to define a Values path for that knob. This way, all the config file knobs are dead simple in how we construct them: svc.header.knob

makes sense - and I really like the idea of translating effectively 1:1 from olso.config to the values.yaml, not only does it provide a uniform base but also compatibility with much of the upstream documentaion with only the need to explain how it's structured. At a (much) later date we could even consider automating such a process to keep in sync with projects as they add and remove options.

alanmeadows · 2017-01-04T00:17:12Z

@@ -0,0 +1,3 @@
+[database]
+connection = mysql+pymysql://{{ .Values.database.cinder_user }}:{{ .Values.database.cinder_password }}@{{ .Values.database.address }}:{{ .Values.database.port }}/{{ .Values.database.cinder_database_name }}


I want to eventually apply the same common lookups or definition patterns for the database, rabbit, and memcache -- I just haven't had an opportunity yet to define an example so this is fine for now.

In our case, I am fine with repeating this values data in every chart as long as its the exact same pattern - our philosophy is with a mildly simplistic script, an environmental yaml file can be used as input and generate an input file for every chart as input, allowing us to duplicate input very easily and these common trees like endpoints, keystone, database, memcache, and so on can be replicated to each chart without pulling in globals or a parent chart.

alanmeadows · 2017-01-04T00:22:16Z

+os_region_name = {{ .Values.keystone.cinder_region_name }}
+
+[keystone_authtoken]
+auth_uri = {{ .Values.keystone.auth_uri }}


could use:

auth_url = {{ include "endpoint_keystone_internal" . }}
and
auth_uri = {{ include "endpoint_keystone_admin" . }}

as these are now in common. Relies on an endpoint definition for keystone in this charts values.yaml.

I really wanted this to be {{ tuple "admin" . | include "endpoint_keystone" }} or {{ tuple "public" . | include "endpoint_keystone" }} but this proved too extreme for gotpl skills. We'll get there.

alanmeadows · 2017-01-04T00:23:52Z

+set -ex
+export HOME=/tmp
+
+ansible localhost -vvv -m mysql_db -a "login_host='{{ .Values.database.address }}' login_port='{{ .Values.database.port }}' login_user='{{ .Values.database.root_user }}' login_password='{{ .Values.database.root_password }}' name='{{ .Values.database.cinder_database_name }}'"


Line breaks if you could - these scripts give me the willies.

alanmeadows · 2017-01-04T00:25:19Z

+      restartPolicy: OnFailure
+      containers:
+        - name: cinder-ks-endpoints-v1-admin
+{{ include "container_ks_endpoint" . | indent 10 }}


I don't mind this approach but we've got to reduce this big monster of a definition here. See above?

Yeah, I'm not proud of this, though Cinder has by far the most endpoints of any service I can think of - I'll get it refactored and sorted asap.

alanmeadows · 2017-01-04T00:29:24Z

+          - /tmp/start.sh
+          ports:
+            - containerPort: {{ .Values.service.api.port }}
+          readinessProbe:


Eventually we should tackle how we can generically do more intelligent readinessProbe's for each service, in the vein of what human operators do to validate openstack services are functioning:

nova list cinder list neutron net-list

of course without the client needing to be involved, so the right curl, with simple authentication, perhaps a keystone bootstrap token. Just spit balling here--not for this PR obviously.

keystone bootstrap token's are pretty scary things :/ but we can resolve this pretty easily I think using an exec probe and an account with a suitable role? Though that in turn may be overly complex.

Sure I think we can come up with something suitable. I will create an issue to track and discuss.

DTadrzak

A few changes.

DTadrzak · 2017-01-05T10:58:41Z

+# limitations under the License.
+
+set -ex
+


We shouldn't create script which consists of only 1 command inside let's call it directly in manifest.

command: - cinder-api - --config-dir - /etc/cinder/conf

DTadrzak · 2017-01-05T10:59:33Z

@@ -0,0 +1,19 @@
+#!/bin/bash


DTadrzak · 2017-01-05T11:04:26Z

+  node_selector_value: enabled
+
+images:
+  dep_check: quay.io/stackanetes/kubernetes-entrypoint:v0.1.0


dep_check and pull_policy should be move to global/common.

We can address the pull_policy (and entrypoint) globalization as part of the spec we need to pull together as a group @DTadrzak. Your stance on this is well noted and appreciated ;-)

intlabs · 2017-01-09T02:26:36Z

+      api: 8776
+  cinderv2:
+    name: cinder
+    hosts:


@alanmeadows, I'm struggling to work out how to solve the issue of versioned endpoints elegantly. The way you have written this makes the most sense from an OpenStack project perspective, but it would actually be more appropriate to swap the service 'name' (eg keystone:) and 'type' key:pairs (eg 'identity'). Though I'm loathe to do this as it would be very confusing to most operators I think? As an alternative, I'm proposing to add a new key 'name', and when not present the templates will default to the sections parent as it uses currently - though this will make for more complex templates. Do you (or anyone else!) have any thoughts on this?

This decision will affect the way that sections like the following are laid out and interacted with:

https://github.com/portdirect/openstack-helm/blob/cinder/cinder/templates/job-ks-endpoints.yaml.yaml#L35

https://github.com/portdirect/openstack-helm/blob/heat/common/templates/_endpoints.tpl#L95-L132

Pete,

In fact all the other lookup routes I've contributed to leverage the type as a first class citizen, using the exact structure your referring to, e.g.

identity:
...
compute:
...

Letting the operator swap components.

I'm all for making this change. Since other charts are using it as is it makes sense to do this sweeping change as part of coming up with the full 'chart architecture document'

For an example of something I contributed to see which is where I'm trying to go:

https://github.com/openstack/cookbook-openstack-common/blob/master/attributes/default.rb
https://github.com/openstack/cookbook-openstack-common/blob/master/libraries/endpoints.rb

That makes total sense - I'll introduce a small hack into the cinder templates (as this is the only service affected in the PoC) and we can address the issue as part of the 'chart architecture document'. Cheers

intlabs · 2017-01-10T02:20:49Z

@alanmeadows I have not been able to confirm Ceph Operation as I do not currently have access to a cluster with Ceph Support. However, I think it should be operational as this mirrors both Glance, and borrows liberally from Stackanetes Ceph Templates.

alanmeadows · 2017-01-10T02:25:19Z

No problem @intlabs, I can verify early tomorrow morning, and pending that LGTM! Nice work.

alanmeadows · 2017-01-10T19:02:13Z

This has been "validated" in our openstack-helm full stack lab with a few minor updates requested of @intlabs. I eagerly await our gating tests getting setup (in progress) to avoid this type of validation.

+--------------------------------------+-----------+--------+------+-------------+----------+-------------+
|                  ID                  |   Status  |  Name  | Size | Volume Type | Bootable | Attached to |
+--------------------------------------+-----------+--------+------+-------------+----------+-------------+
| 0f57e259-bf9d-4743-a871-d9ed5e393f40 | available | test50 |  1   |      -      |  false   |             |
| 70683904-5364-4268-aa08-e0a126b04d97 | available | test10 |  1   |      -      |  false   |             |
+--------------------------------------+-----------+--------+------+-------------+----------+-------------+

Cinder WIP

This work is dependant on the Common Chart elements introduced with the Heat PR, and should not be merged prior to #77

…ersions This commmit addresses issues with the endpoint layout in the values.yaml As a result it does for now not use the common functions for some tasks.

This commit adds support for a Ceph RBD backend

This brings Cinder in line with the following PRs: * #98 * #97

Untill the endpoint values.yaml is brought into line with other services, we need to use the old method of setting the keystone URL in cinder.conf.

alanmeadows suggested changes Jan 4, 2017

View reviewed changes

DTadrzak suggested changes Jan 5, 2017

View reviewed changes

intlabs commented Jan 9, 2017

View reviewed changes

portdirect and others added 9 commits January 10, 2017 19:10

OpenStack Cinder WIP

adbe8f7

Cinder WIP

Cinder Refactor WIP

4479a0d

This work is dependant on the Common Chart elements introduced with the Heat PR, and should not be merged prior to #77

WIP Update cinder Endpoint and service creation to support multiple v…

1e94b12

…ersions This commmit addresses issues with the endpoint layout in the values.yaml As a result it does for now not use the common functions for some tasks.

Cinder Ceph RBD backend

f34441a

This commit adds support for a Ceph RBD backend

Cinder add policy.json

3f491f5

Add configmap hashing and rolling update to cinder

3163f1e

This brings Cinder in line with the following PRs: * #98 * #97

Fix Cinder conf Keystone Endpoint

a5688da

Cinder revert cinder.conf keystone endpoints

192bcd6

Untill the endpoint values.yaml is brought into line with other services, we need to use the old method of setting the keystone URL in cinder.conf.

Update Cinder PR to enable Ceph Backend

831f63d

alanmeadows approved these changes Jan 10, 2017

View reviewed changes

alanmeadows merged commit bab88b8 into att-comdev:master Jan 10, 2017

intlabs deleted the cinder branch January 22, 2017 22:00

		@@ -0,0 +1,3 @@
		[database]
		connection = mysql+pymysql://{{ .Values.database.cinder_user }}:{{ .Values.database.cinder_password }}@{{ .Values.database.address }}:{{ .Values.database.port }}/{{ .Values.database.cinder_database_name }}

Conversation

intlabs commented Jan 2, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alanmeadows Jan 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alanmeadows Jan 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alanmeadows Jan 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alanmeadows Jan 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

DTadrzak left a comment

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!

intlabs Jan 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

intlabs Jan 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

intlabs commented Jan 10, 2017

Uh oh!

alanmeadows commented Jan 10, 2017

Uh oh!

alanmeadows commented Jan 10, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

intlabs commented Jan 2, 2017 •

edited

Loading

alanmeadows Jan 4, 2017 •

edited

Loading

alanmeadows Jan 4, 2017 •

edited

Loading

alanmeadows Jan 4, 2017 •

edited

Loading

alanmeadows Jan 4, 2017 •

edited

Loading

intlabs Jan 9, 2017 •

edited

Loading

intlabs Jan 9, 2017 •

edited

Loading