Bug 1358257 - Container auto-tagging from labels breaks refresh on labels with empty value
Summary: Container auto-tagging from labels breaks refresh on labels with empty value
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Red Hat CloudForms Management Engine
Classification: Red Hat
Component: Providers
Version: 5.6.0
Hardware: Unspecified
OS: Unspecified
high
high
Target Milestone: GA
: 5.7.0
Assignee: Beni Paskin-Cherniavsky
QA Contact: Einat Pacifici
URL:
Whiteboard: container
Depends On:
Blocks: 1358303
TreeView+ depends on / blocked
 
Reported: 2016-07-20 11:42 UTC by Beni Paskin-Cherniavsky
Modified: 2018-04-17 08:04 UTC (History)
7 users (show)

Fixed In Version: 5.7.0.0
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
: 1358303 (view as bug list)
Environment:
Last Closed: 2017-01-11 19:58:44 UTC
Category: ---
Cloudforms Team: ---
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Github 9713 0 None None None 2016-07-20 11:42:20 UTC

Description Beni Paskin-Cherniavsky 2016-07-20 11:42:21 UTC
Description of problem:

https://github.com/ManageIQ/manageiq/issues/9713

Manually editing ContainerLabelTagMapping table (which has no UI at this point)
to create an *any-value* mapping for some label key (see steps below),
works given `foo=bar` labels but causes errors during refresh if it encounters a empty-value `foo=` label.

[Kubernetes labels with can legally have an empty value: http://kubernetes.io/docs/user-guide/labels/#syntax-and-character-set]

It does not happen if there is also *specific-value* mapping for the empty value.  Having such a mapping for every any-value mapping is a viable workaround on 5.6.0.


Steps to Reproduce:
1.  Create a category and an any-value mapping for label key `foo` by running in rails console:

        cat = Classification.create_category!(name: "label_foo", description: "Automatically set from `foo` kubernetes label", read_only: true, single_value: true)
        ContainerLabelTagMapping.create!(label_name: 'foo', label_value: nil, tag: cat.tag)
        ContainerLabelTagMapping.drop_cache

2.  Create empty `foo=` label on some entity e.g. pod.
    `kubectl label`/`oc label` won't let you assign a `foo=` label, but it's possible with `oc edit`.  
    Easiet way is first running creating non-empty label, then editing:

        $ kubectl label pod heapster-irfy9 foo=bar
        $ kubectl edit pod heapster-irfy9

        ...
        metadata:
          ...
          labels:
            foo: ""     # <--- replace here bar with ""
            metrics-infra: heapster
        ...

3. Refresh the kubernetes/openshift provider in CFME.

Actual results:
Provider refresh fails, last status shows "Validation failed: Description can't be blank, Name can't be blank"
Stacktrace (reproduced in rails console): https://github.com/ManageIQ/manageiq/issues/9713#issuecomment-231585198
It seems it aborts out of EmsRefresh.refresh entirely.

Expected results:
Refresh should succeed.
I see 2 plausible behaviors, not sure which is better:
 1. don't assign any tags for empty-value labels (`foo=` label treated same as not having any `foo` label)
 2. create `Foo : <emtpy value>` tags

I semi-arbitrarily intend to do (1), would love to hear preferences either way.

Comment 3 Beni Paskin-Cherniavsky 2016-07-25 21:02:18 UTC
Update: after discussion on PR #9747, we switched to behavior 2 
(create `Foo : <emtpy value>` tags).

Comment 4 CFME Bot 2016-07-26 13:25:54 UTC
New commit detected on ManageIQ/manageiq/master:
https://github.com/ManageIQ/manageiq/commit/90155b96186a036c5e406ee5308921521c524535

commit 90155b96186a036c5e406ee5308921521c524535
Author:     Beni Cherniavsky-Paskin <cben>
AuthorDate: Mon Jul 25 20:00:51 2016 +0300
Commit:     Beni Cherniavsky-Paskin <cben>
CommitDate: Mon Jul 25 20:02:05 2016 +0300

    Fix auto-tagging to handle empty-value labels
    
    Given any-value mapping `foo` -> "Foo" category,
    `foo=` label will create "Foo : <empty value>" tag.
    
    Fixes #9713,
    https://bugzilla.redhat.com/show_bug.cgi?id=1358257
    https://bugzilla.redhat.com/show_bug.cgi?id=1358303

 app/models/container_label_tag_mapping.rb       | 11 +++++++++--
 spec/models/container_label_tag_mapping_spec.rb |  7 ++++---
 2 files changed, 13 insertions(+), 5 deletions(-)

Comment 5 Beni Paskin-Cherniavsky 2016-07-27 13:31:59 UTC
Forgot to mention in description:
after using `rails console` to create the mapping in the DB, 
should restart CFME to reload it.
(it's cached in memory indefinitely; the `ContainerLabelTagMapping.drop_cache` line above affects the rails console process but not an already running instance).

Comment 6 CFME Bot 2016-10-28 13:11:28 UTC
New commit detected on ManageIQ/manageiq/master:
https://github.com/ManageIQ/manageiq/commit/593a0cd8930e3ceb093da1fe4d44301a0e448f9d

commit 593a0cd8930e3ceb093da1fe4d44301a0e448f9d
Author:     Beni Cherniavsky-Paskin <cben>
AuthorDate: Thu Oct 13 21:43:40 2016 +0300
Commit:     Beni Cherniavsky-Paskin <cben>
CommitDate: Wed Oct 26 20:04:51 2016 +0300

    Do not create tags for empty-value labels
    
    This reverts decision on
    https://github.com/ManageIQ/manageiq/issues/9713#issuecomment-233929144
    https://bugzilla.redhat.com/show_bug.cgi?id=1358257
    back to "behavior 1" of ignoring labels with empty value.
    
    This only affects any-value mappings.
    A specific value='' mapping would still be honored.

 app/models/container_label_tag_mapping.rb       | 14 ++++++--------
 spec/models/container_label_tag_mapping_spec.rb |  6 ++----
 2 files changed, 8 insertions(+), 12 deletions(-)

Comment 7 Beni Paskin-Cherniavsky 2016-11-03 19:36:02 UTC
`oc label`, `oc edit` apparently gives error on empty label value in newer openshift (v3.3).  Found a different way using curl.
And there is now UI to configure mappings (https://github.com/ManageIQ/manageiq/pull/11591).
And expected behavior changed to no tag.

==> UPDATED INSTRUCTIONS:

(Steps 2 & 3 below could be skipped but then it's hard to know step 5 worked.)

 1. Top right menu -> Configuration -> Region 0 in explorer -> Map Tags tab.
    Add a mapping rule from Entity type <ALL>, Label "foo" to category "Foo Whatever".

 2. Create `foo=bar` label on some entity, let's say a pod:

    $ oc label pod --namespace=default router-1-l67gi foo=bar

 3. Refresh the provider.
    Expected results:
    - Provider screen shows Last Refresh Status "successful" with recent time (e.g. 1 minute ago).
    - The entity's screen shows "foo | bar"  label in Labels table AND "Foo Whatever : bar" tag in "Smart Management" table.

 4. Change the label to empty value:

    $ oc proxy --port=9443 &
    [1]
    Starting to serve on 127.0.0.1:9443
    $ curl -XPATCH  -H "Content-Type: application/strategic-merge-patch+son" http://localhost:8080/api/v1/namespaces/default/pods/router-1-l67gi -d '{"metadata":{"labels":{"foo":""}}}' | grep --after=5 labels
    $ fg
    oc proxy --port=9443
    ^C

 5. Refresh the provider.

    Expected results:
    - Provider screen shows Last Refresh Status "successful" with recent time (e.g. 0 minute ago).
    - The entity's screen shows "foo |"  empty label in Labels table AND NO TAG in "Smart Management" table.

[If you want to label other entity types, the URL structure differs, search "PATCH" in http://kubernetes.io/docs/api-reference/v1/operations/.  To label a Project, use `oc label namespace NAME` and /api/v1/namespaces/NAME.]

Comment 8 Beni Paskin-Cherniavsky 2016-11-07 13:19:07 UTC
Corrections to step 4:
- the oc proxy port should match port used in curl
- s/+son/+json/

 4. Change the label to empty value:

    $ oc proxy --port=8080 &
    [1]
    Starting to serve on 127.0.0.1:9443
    $ curl -XPATCH  -H "Content-Type: application/strategic-merge-patch+json" http://localhost:8080/api/v1/namespaces/default/pods/router-1-l67gi -d '{"metadata":{"labels":{"foo":""}}}' | grep --after=5 labels
    $ fg
    oc proxy --port=8080
    ^C


P.S. To label projects, use url of the form http://localhost:8080/api/v1/namespaces/NAME

Comment 9 Einat Pacifici 2016-11-08 08:57:49 UTC
Verified. 
Followed steps in comment 7 and comment 8 
As below: 
$ oc proxy --port=9443 &

$curl -XPATCH  -H "Content-Type: application/strategic-merge-patch+json" http://localhost:9443/api/v1/namespaces/hello4 -d '{"metadata":{"labels":{"dept":""}}}' | grep --after=5 labels
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed

Viewing Project in CFME, for Project=hello4, Labels table has empty values for dept.


Note You need to log in before you can comment on or make changes to this bug.