Remove automatic assignment of gpu label

[Brent Rowsell]

The following commit, https://review.opendev.org/#/c/666511/ automatically add the intelgpu label to any host that has a intel gpu.

This is incorrect. The decision whether to leverage the device on the host needs to be a user decision not the system. Please remove the automated assignment of the labels.

[Ghada Khalil (gkhalil)]

Marking as stx.3.0 / high priority as this needs to be resolved asap so that the QAT & FPGA feature code can be updated and merged as this code builds on the same premise.

This was somehow missed during the code review phase.

[Ran An (an.ran)]

Hi Brent,
according to commit https://review.opendev.org/666511/, the label "intelgpu: enabled" will only be assigned to nodes if all of the following conditions are met:
1. the user deployed intel-gpu-plugin daemon set. (override "k8s_plugins" adding "intel-gpu-plugin:intelgpu=enabled").
2. the nodes was detected intel gpu.

I think the first condition has already met your requirement.

[Brent Rowsell (brent-rowsell)]

I do not agree. The commit blindly assumes that the user would want to make gpu's on any server available to k8s. The user has to be given the opportunity to explicitly declare which servers they wish to deploy with gpu's.

[Ran An (an.ran)]

If the user here means "normal users" (K8S tenants), they can choose to require the GPU resource or not in their pod YAML (of "application" workloads). It doesn't mean they have the permission of deciding whether or not the node with the GPU plugin should be enabled by the label.

------------------------------------------------------------------------------
On the other hand, if user here you meant "sysadmin", yes, we can modify this patch so that sysadmin can do it manually based on his/her needs.

[Brent Rowsell (brent-rowsell)]

It is up to the user (system administrator) to assign label to the nodes they want to assign gpu workloads to.

[Ran An (an.ran)]

1. Currently, the system administrator could remove the label by cmd "system host-label-remove" if he/she want to keep a node away from gpu workloads.
Does this meet the requirement?

2. for the labels of device plugins, following checks are required before they are assigned to nodes:
  1). ensure the node support specify HW.
  2). ensure the system support specify device plugin.

if we don't check and assign plugin labels at provisioning time (automatically), we need to add a new sysinv command for the administrator to do above checks before assign k8s label to node.

[Matt Peters (mpeters-wrs)]

In addition to the undesired operational behavior, there are technical issues with the current implementation. Currently /etc/platform/enabled_kube_plugins is only set at bootstrap, which means it will only be created on the bootstrap controller. A switch of activity to the other controller will have the sysinv-conductor running on controller-1 and will therefore not automatically label any additional nodes.

Furthermore, the user will not be able to suppress the label configuration as you describe above because a restart of the sysinv-agent will trigger any removed labels to be repopulated.

[Ran An (an.ran)]

thanks for Matt's comment, it make sense. I'll remove the automatic label assign.

in addition, it would solve the file sync issue if file "enabled_kube_plugins" is set under path "/opt/extension/" (a drbd path), right?

[Matt Peters (mpeters-wrs)]

Yes a DRBD path would be replicated and available on the active controller.

[Ghada Khalil (gkhalil)]

Hi Ran, can you please put up the review for this as soon as possible? We would like this code simplification to go in so that the QAT and FPGA code changes are updated accordingly before they merge.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.opendev.org/691699

[Ran An (an.ran)]

Hi Ghada, based on committed patch above, there are no changes of QAT&FPGA codes in sysinv (getting plugin labels part).

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.opendev.org/691801

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.opendev.org/693222

[Ghada Khalil (gkhalil)]

Based on discussion with Ran during the community meeting on 11/6, the immediate focus will be on the following two commits:
https://review.opendev.org/#/c/693222/ -- revert the automatic addition of labels in sysinv
https://review.opendev.org/#/c/691801/ -- address issues w/ controller swact & restore

As a future enhancement (longer term), https://review.opendev.org/#/c/691699/ will add semantic checks to ensure the user is adding the labels on nodes that actually have the required hardware.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/693222
Committed: https://git.openstack.org/cgit/starlingx/config/commit/?id=93320956b4d150904fc3c988f9ea02619566b3f6
Submitter: Zuul
Branch: master

commit 93320956b4d150904fc3c988f9ea02619566b3f6
Author: SidneyAn <email address hidden>
Date: Thu Nov 7 00:09:45 2019 +0800

    Revert "add support for intel gpu device plugin"

    This reverts commit 023be74256c09ec84dc547443af07a7bc73cce57.
    according to the architectural direction, labels should be
    assigned by user manually instead of automaticly.

    The HW detect and k8s_plugins configuration check which can
    avoid user to assign plugin labels in imporper node will be
    kept as semantic checks of labels.
    it will be implemeted at https://review.opendev.org/#/c/691699/

    Change-Id: I601dc84264864eb5b292edd47f5c53668193f994
    Closes-bug: 1849326

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/691801
Committed: https://git.openstack.org/cgit/starlingx/ansible-playbooks/commit/?id=52f924b547ae2a50bb6458870ae985907ec81aef
Submitter: Zuul
Branch: master

commit 52f924b547ae2a50bb6458870ae985907ec81aef
Author: SidneyAn <email address hidden>
Date: Tue Oct 29 10:55:19 2019 +0800

    enhance user defined "k8s_device_plugins": support controller switch and system restore

    this commit include following changes:
    1. relocate file "enabled_kube_plugins" on drbd path. file "enabled_kube_plugins"
    record the plugins enabled by administrator which is only set at bootstrap. to avoid
    missing this file after controller switch, it should be set in folder monitored by drbd.

    2. set fact "k8s_plugins" during restore process according to file
    "enabled_kube_plugins" in backup tar file.

    Change-Id: Idea0034516d844f8a6a6772b8f4249fdfba86397
    Partial-bug: 1849326
    Signed-off-by: SidneyAn <email address hidden>