Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
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: ---


Attachments (Terms of Use)


Links
System ID Priority Status Summary Last Updated
Github 9713 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@redhat.com>
AuthorDate: Mon Jul 25 20:00:51 2016 +0300
Commit:     Beni Cherniavsky-Paskin <cben@redhat.com>
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@redhat.com>
AuthorDate: Thu Oct 13 21:43:40 2016 +0300
Commit:     Beni Cherniavsky-Paskin <cben@redhat.com>
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.