Issue #207 Stnd control plane node issue

[renmak]

What is the purpose of this pull request?: Updating Nova label name and value to align with how labels are used in Neutron

What issue does this pull request address?: Fixes issue #207. Also this new PR replaces previous PR #285

Notes for reviewers to consider: Please look at comment from Allan in PR #285. My new changes in this PR are based on his comment.

Specific reviewers for pull request: @alanmeadows @v1k0d3n

[gardlt]

should we try and propagate this changes to the other charts as well not just the keystone and compute ones. @intlabs @v1k0d3n @renmak

[wilkers-steve]

@renmak Thanks for taking this on. I looked through the comments on PR #285, and this gets us closer. However, I think it doesn't quite get us to where @alanmeadows suggested for the labels for nova. Instead of having a blanket approach to labels under a compute and control tree, I think the goal is to split this out per service that Nova runs. So instead of compute: or control:, it'd be conductor:, compute:, scheduler:, and so on -- at least that's my understand. Could you share your thoughts on that @alanmeadows?

[renmak]

@wilkers-steve @v1k0d3n @alanmeadows So if we go by services, it would look like following

consoleauth: openstack-compute-node scheduler: openstack-control-plane conductor: openstack-control-plane compute: openstack-control-plane network: api:

We will refer to above key value as {{ .Values.labels.scheduler.node_selector_key }}

Is this what you guys have in mind? Thanks

[alanmeadows]

This looks good, except I agree with steve, lets just break it out, and simultaneously, achieve parity with neutron and its agent approach:

labels:
  agent:
    compute:
      node_sele...
  osapi: 
    node_sele...
  conductor:
    node_sele...

also, even though its outside this pull request, can we fix replicas as its now a bit strange with the removal of control and is a simple fix. See any other chart for how replicas has shaped up (except in nova)

replicas:
  api: ..
  conductor: ..
...and so on...

[renmak]

@alanmeadows @wilkers-steve @v1k0d3n @intlabs

Thanks Alan for feedback. So based on your feedback, this is what i came up with. Please correct me if this is incorrect.

So based on openstack-helm/nova/templates/ each, daemonset and deployment have been addressed below.

labels:
  agent:
    compute: 
      node_selector_key: openstack-compute-plane
      node_selector_value: enabled
    libvert:
      node_selector_key: openstack-control-plane
      node_selector_value: enabled
  conductor:
    node_selector_key: openstack-control-plane
    node_selector_value: enabled
  consoleauth:
    node_selector_key: openstack-control-plane
    node_selector_value: enabled
  scheduler:
    node_selector_key: openstack-control-plane
    node_selector_value: enabled
  osapi:
    node_selector_key: openstack-control-plane
    node_selector_value: enabled
  api-metadata:
    node_selector_key: openstack-control-plane
    node_selector_value: enabled
  server:
    node_selector_key: openstack-control-plane
    node_selector_value: enabled

[alanmeadows]

Libvirt isn't really an agent, but this could work for now

labels:
  agent:
    compute: 
      node_selector_key: openstack-compute-node
      node_selector_value: enabled
    libvirt:
      node_selector_key: openstack-compute-node
      node_selector_value: enabled

Note the openstack-compute-node for agent processes (those that run on compute hosts vs control plane)

All the other definitions are fine, but what is server: here for nova?

[renmak]

k that's my misunderstanding. server: should not be there.

But that brings up a question. We have total 5 jobs. db-init, db-sync, ks-endpoints, ks-service and ks-user. Each of these jobs refers to openstack-control-plane.

Which label do we use for these jobs?

[alanmeadows]

I would just add the below and have them refer to this. I do not think we have standardized this approach (allowing jobs (as a holistic category) to target their label), but a separate pull request could introduce this pattern everywhere:

labels:
  job:
    node_selector_key: openstack-control-plane
    node_selector_value: enabled

Defining a label for each job may be overkill.

[renmak]

Thanks for clarification Alan.

As per your suggestions on Replicas, here is what i was planning.

replicas:
  api: 1    #for both api-osapi & api-metadata. Should i also 
  conductor: 1
  consoleauth: 1
  scheduler: 1 

I will make needed code changes and update this PR.

[alanmeadows]

All sounds fine except I would ensure metadata has its own replica declaration, much like api-metadata does. This gives you parity with labels.

Also, if we're calling it "osapi" consistently, lets use that for replicas as well.

The goal is consistent naming between the names within the deployment manifests themselves, the replica fields, and the label fields so that all three match as closely as possible.

[renmak]

I will squash all these commits into one final commit

[renmak]

Thanks for review and catching bug. I will work on this today and get code changes in.

[renmak]

K so update one final commit with all changes. Thanks @v1k0d3n @wilkers-steve @larryrensing for reviews.

[wilkers-steve]

@renmak This looks good. Thanks for fixing this. Pulled it down locally and everything runs fine. I'll go ahead and merge this in once @larryrensing signs off on the changes being satisfactory

[intlabs]

LGTM :) I'd merge @wilkers-steve