Bug 1596548

Summary:	Editing(Adding) Output Image Labels doesn't work in OCP 3.6
Product:	OpenShift Container Platform	Reporter:	Min Woo Park <mpark>
Component:	Build	Assignee:	Jan Wozniak <jwozniak>
Status:	CLOSED ERRATA	QA Contact:	Wenjing Zheng <wzheng>
Severity:	high	Docs Contact:
Priority:	unspecified
Version:	3.6.0	CC:	aos-bugs, bparees, mpark
Target Milestone:	---
Target Release:	3.6.z
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
Fixed In Version:		Doc Type:	Bug Fix
Doc Text:	fix user-defined output image labels override to behave consistently with other versions	Story Points:	---
Clone Of:		Environment:
Last Closed:	2018-07-24 04:30:19 UTC	Type:	Bug
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:

Description Min Woo Park 2018-06-29 08:37:05 UTC

Description of problem:

Editing(Adding) Output Image Labels doesn't work on OCP 3.6
It works well on OCP 3.5 and 3.9

My customer wants to add(or change) the labels during S2I build on OCP 3.6.

reference doc  
[1] https://docs.openshift.com/container-platform/3.6/dev_guide/builds/build_output.html#output-image-environment-variables

Version-Release number of selected component (if applicable):


How reproducible:
I tested with eap-app image from doc
[2] https://access.redhat.com/documentation/en-us/red_hat_jboss_enterprise_application_platform/7.1/html-single/red_hat_jboss_enterprise_application_platform_for_openshift/#deploy_eap_s2i

Steps to Reproduce:
1. Deploy JBoss EAP S2I Application to OCP
2. Edit bc/eap-app like,
~~~~~~~~~~~~~
spec:
  nodeSelector: null
  output:
    imageLabels:
    - name: io.openshift.build.commit.author
      value: Ben Parees <bparees.github.com>
    - name: io.openshift.build.commit.message
      value: 'Merge pull request #187 from Gl4di4torRr/master'
    to:
      kind: ImageStreamTag
      name: eap-app:latest
~~~~~~~~~~~~~
3. 

Actual results:
- Only on OCP 3.6

(before new build)
# oc describe image xxx | grep "io.openshift.build.commit"
		io.openshift.build.commit.author=Tomaz Cerar <tomaz.cerar>
		io.openshift.build.commit.date=Thu Dec 14 01:05:08 2017 +0100
		io.openshift.build.commit.id=465e5a2e7900cdb744740745f3d65a110d6e9962
		io.openshift.build.commit.message=Add generated docs
		io.openshift.build.commit.ref=7.1.0.GA

(after new build)
# oc describe image yyy | grep "io.openshift.build.commit"
		io.openshift.build.commit.author=Tomaz Cerar <tomaz.cerar>
		io.openshift.build.commit.date=Thu Dec 14 01:05:08 2017 +0100
		io.openshift.build.commit.id=465e5a2e7900cdb744740745f3d65a110d6e9962
		io.openshift.build.commit.message=Add generated docs
		io.openshift.build.commit.ref=7.1.0.GA

Expected results:
- tested same thing on OCP3.5 and 3.9

(before new build)
# oc describe image xxx | grep "io.openshift.build.commit"
		io.openshift.build.commit.author=Tomaz Cerar <tomaz.cerar>
		io.openshift.build.commit.date=Thu Dec 14 01:05:08 2017 +0100
		io.openshift.build.commit.id=465e5a2e7900cdb744740745f3d65a110d6e9962
		io.openshift.build.commit.message=Add generated docs
		io.openshift.build.commit.ref=7.1.0.GA

(after new build)
# oc describe image yyy | grep "io.openshift.build.commit"
		io.openshift.build.commit.author=Ben Parees <bparees.github.com>
		io.openshift.build.commit.date=Thu Dec 14 01:05:08 2017 +0100
		io.openshift.build.commit.id=465e5a2e7900cdb744740745f3d65a110d6e9962
		io.openshift.build.commit.message=Merge pull request #187 from Gl4di4torRr/master
		io.openshift.build.commit.ref=7.1.0.GA

Additional info:

Comment 1 Ben Parees 2018-06-29 14:54:55 UTC

Are you able to add new labels, just not overwrite labels that are injected by openshift?

Comment 2 Ben Parees 2018-06-29 14:58:02 UTC

I think the issue here is that s2i is overwriting the user injected labels when they have the same keys.  Starting in 3.7 we handled the labels different.  I'm a bit surprised it worked in 3.5.

Comment 3 Min Woo Park 2018-07-02 00:28:48 UTC

Hi Ben,

Adding new labels works well on OCP 3.6 / 3.5 / 3.9
Editing(Overwriting) the labels doesn't work on OCP 3.6 but works well on OCP 3.5 / 3.9

Is there any workaround or hotfix for OCP 3.6 ??

The test result,

- bc/eap-app
~~~~~~~~~~~~~~~~~~~
spec:
  failedBuildsHistoryLimit: 5
  nodeSelector: null
  output:
    imageLabels:
    - name: io.openshift.build.commit.author   <-------------- Editing(Overwriting) the labels
      value: Ben Parees <bparees.github.com>
    - name: io.openshift.build.commit.message  <-------------- Editing(Overwriting) the labels
      value: 'Merge pull request #187 from Gl4di4torRr/master'
    - name: testusername                       <-------------- Adding new labels
      value: 'Min Woo Park'
~~~~~~~~~~~~~~~~~~~

** OCP 3.6

  * docker-registry.default.svc:5000/eap-demo/eap-app@sha256:8fc25eca06a70aef72a327956d777eee5392df153e5ce964f47128e020b10a06
      2 minutes ago
    docker-registry.default.svc:5000/eap-demo/eap-app@sha256:da3c9a1c7a04aa2ea39af807d14f5f9138f3803f396abbfa752e69ae487d6dca
      2 days ago

(before)
# oc describe image sha256:da3c9a1c7a04aa2ea39af807d14f5f9138f3803f396abbfa752e69ae487d6dca | grep test
# oc describe image sha256:da3c9a1c7a04aa2ea39af807d14f5f9138f3803f396abbfa752e69ae487d6dca | grep commit
		io.openshift.build.commit.author=Tomaz Cerar <tomaz.cerar>
		io.openshift.build.commit.date=Thu Dec 14 01:05:08 2017 +0100
		io.openshift.build.commit.id=465e5a2e7900cdb744740745f3d65a110d6e9962
		io.openshift.build.commit.message=Add generated docs
		io.openshift.build.commit.ref=HEAD

(after)
# oc describe image sha256:8fc25eca06a70aef72a327956d777eee5392df153e5ce964f47128e020b10a06 | grep test
		testusername=Min Woo Park                                              <-------------- Added new label
# oc describe image sha256:8fc25eca06a70aef72a327956d777eee5392df153e5ce964f47128e020b10a06 | grep commit
		io.openshift.build.commit.author=Tomaz Cerar <tomaz.cerar>   <-------------- NOT Edited(Overwrote) the label
		io.openshift.build.commit.date=Thu Dec 14 01:05:08 2017 +0100
		io.openshift.build.commit.id=465e5a2e7900cdb744740745f3d65a110d6e9962
		io.openshift.build.commit.message=Add generated docs                   <-------------- NOT Edited(Overwrote) the labels
		io.openshift.build.commit.ref=HEAD

-----

** OCP 3.5

  * 172.30.82.27:5000/eap-demo/eap-app@sha256:396d50376cbbbde7e6043e3ff2bfc9e0675cee0b3e6beed04ef8e2ed44df3cc0
      31 seconds ago
    172.30.82.27:5000/eap-demo/eap-app@sha256:d6687c93f92e308d3713aa53dec9e75969672556ac334dd22462a00740682852
      3 weeks ago

(before)
# oc describe image sha256:d6687c93f92e308d3713aa53dec9e75969672556ac334dd22462a00740682852 | grep test
# oc describe image sha256:d6687c93f92e308d3713aa53dec9e75969672556ac334dd22462a00740682852 | grep commit
		io.openshift.build.commit.author=Tomaz Cerar <tomaz.cerar>
		io.openshift.build.commit.date=Thu Dec 14 01:05:08 2017 +0100
		io.openshift.build.commit.id=465e5a2e7900cdb744740745f3d65a110d6e9962
		io.openshift.build.commit.message=Add generated docs
		io.openshift.build.commit.ref=HEAD

(after) 
# oc describe image sha256:396d50376cbbbde7e6043e3ff2bfc9e0675cee0b3e6beed04ef8e2ed44df3cc0 | grep test
		testusername=Min Woo Park                                                          <-------------- Added new label
# oc describe image sha256:396d50376cbbbde7e6043e3ff2bfc9e0675cee0b3e6beed04ef8e2ed44df3cc0 | grep commit
		io.openshift.build.commit.author=Ben Parees <bparees.github.com>     <-------------- Edited(Overwrote) the label
		io.openshift.build.commit.date=Thu Dec 14 01:05:08 2017 +0100
		io.openshift.build.commit.id=465e5a2e7900cdb744740745f3d65a110d6e9962
		io.openshift.build.commit.message=Merge pull request #187 from Gl4di4torRr/master  <-------------- Edited(Overwrote) the label
		io.openshift.build.commit.ref=HEAD

-----

** OCP 3.9

  * docker-registry.default.svc:5000/eap-demo/eap-app@sha256:e99b0d18af5cf3af9ee81a5c98c35c73b55d28e3e6d4fb2e66571e9c3d38e3c8
      About a minute ago
    docker-registry.default.svc:5000/eap-demo/eap-app@sha256:8bd6507958e90e9a107d56375ffedb0ed3539ff340a8574bd4b6403479fb5ad9
      7 minutes ago

(before)
# oc describe image sha256:8bd6507958e90e9a107d56375ffedb0ed3539ff340a8574bd4b6403479fb5ad9 | grep test
# oc describe image sha256:8bd6507958e90e9a107d56375ffedb0ed3539ff340a8574bd4b6403479fb5ad9 | grep commit
		io.openshift.build.commit.author=Tomaz Cerar <tomaz.cerar>
		io.openshift.build.commit.date=Thu Dec 14 01:05:08 2017 +0100
		io.openshift.build.commit.id=465e5a2e7900cdb744740745f3d65a110d6e9962
		io.openshift.build.commit.message=Add generated docs
		io.openshift.build.commit.ref=7.1.0.GA

(after)
# oc describe image sha256:e99b0d18af5cf3af9ee81a5c98c35c73b55d28e3e6d4fb2e66571e9c3d38e3c8 | grep test
		testusername=Min Woo Park                                                          <-------------- Added new label
# oc describe image sha256:e99b0d18af5cf3af9ee81a5c98c35c73b55d28e3e6d4fb2e66571e9c3d38e3c8 | grep commit
		io.openshift.build.commit.author=Ben Parees <bparees.github.com>     <-------------- Edited(Overwrote) the label
		io.openshift.build.commit.date=Thu Dec 14 01:05:08 2017 +0100
		io.openshift.build.commit.id=465e5a2e7900cdb744740745f3d65a110d6e9962
		io.openshift.build.commit.message=Merge pull request #187 from Gl4di4torRr/master  <-------------- Edited(Overwrote) the label
		io.openshift.build.commit.ref=7.1.0.GA

Thanks,

Comment 4 Ben Parees 2018-07-02 02:13:14 UTC

> Is there any workaround or hotfix for OCP 3.6 ??

We haven't figured out the root cause yet, so no.  The only workaround I can suggest right now would be to simply use a different label instead of trying to overwrite the ones that openshift is injecting, to avoid the problem entirely.

Comment 5 Min Woo Park 2018-07-02 02:24:51 UTC

Hi Ben,

Thanks for your reply.

Can I get the bug URL about the issue that s2i is overwriting the user injected labels when they have the same keys? 
I need to explain about it to my customer.

Thanks,

Comment 6 Ben Parees 2018-07-02 02:32:52 UTC

There is no existing bug.  This is the first report of this issue.

Comment 7 Jan Wozniak 2018-07-02 17:00:20 UTC

a couple of findings from reading the code

- in origin builder 3.6 it does not populate labels from git [1] (3.7 does [2])
- in vendored source-to-image post processing, it merges labels from various sources [3,4]
- I think that if origin populates the labels from git as well, vendored source-to-image post processing will probably keep the populated labels and don't overwrite anything

I will start working on a fix and test the logic



[1] https://github.com/openshift/origin/blob/269e8287d57fc9782a1874d297dfed3c0f62c19b/pkg/build/builder/sti.go#L413-L420

[2] https://github.com/openshift/origin/blob/a62384df654260bb4d6cd7f91946b0250876b503/pkg/build/builder/sti.go#L370-L386

[3] https://github.com/openshift/origin/blob/269e8287d57fc9782a1874d297dfed3c0f62c19b/vendor/github.com/openshift/source-to-image/pkg/build/strategies/sti/postexecutorstep.go#L421-L444

[4] https://github.com/openshift/origin/blob/269e8287d57fc9782a1874d297dfed3c0f62c19b/vendor/github.com/openshift/source-to-image/pkg/util/labels.go#L11-L21

Comment 8 Jan Wozniak 2018-07-03 15:53:30 UTC

I think I know why the 3.5 works and 3.6 doesn't. The mergeLabels function got simplified in 3.6 [1] and despite the order of arguments, the 3.5 mergeLabels [2] function was implemented in the order existingLabels, generatedLabels, configLabels. In 3.7 this is already fixed [3]

From my testing, it looks like a simple fix in origin and simple fix in vendor/source-to-image will suffice. I will prepare the PRs.



[1] https://github.com/openshift/origin/blob/269e8287d57fc9782a1874d297dfed3c0f62c19b/vendor/github.com/openshift/source-to-image/pkg/build/strategies/sti/postexecutorstep.go#L421-L444

[2] https://github.com/openshift/origin/blob/e43c61db765be6ca7ab30427e4dd8a12027307a4/vendor/github.com/openshift/source-to-image/pkg/build/strategies/sti/postexecutorstep.go#L419-L444

[3] https://github.com/openshift/origin/blob/a62384df654260bb4d6cd7f91946b0250876b503/vendor/github.com/openshift/source-to-image/pkg/build/strategies/sti/postexecutorstep.go#L433

Comment 9 Ben Parees 2018-07-03 17:12:34 UTC

yeah sounds like it's just a backport of https://github.com/openshift/source-to-image/pull/812 to the s2i 3.6 branch, and then a re-vendoring into ose 3.6.

Comment 10 Jan Wozniak 2018-07-04 11:44:07 UTC

Godeps seems to be a bit broken for origin-3.6, also the referenced source-to-image dependency was not the tip of openshift-3.6.0 branch so the backport https://github.com/openshift/source-to-image/pull/900 is going to bring a few changes with it.

Created bump(*) in origin, https://github.com/openshift/origin/pull/20205. If this change is ok, will create matching PR to OSE as well. Another approach could be branching out from the last referenced source-to-image commit and applying the backport there. This would bring fewer changes.

Comment 11 Ben Parees 2018-07-09 18:02:11 UTC

https://github.com/openshift/ose/pull/1345

Comment 13 Wenjing Zheng 2018-07-12 09:23:51 UTC

Verified with openshift v3.6.173.0.126:
1. oc new-app ruby~https://github.com/openshift/ruby-ex
2. oc describe images sha256:89ae2f447d0c9858b25d813944e7f5e906290a3a1dff7b33c09347914bdae44a | grep "io.openshift.build.commit"
		io.openshift.build.commit.author=Ben Parees <bparees.github.com>
		io.openshift.build.commit.date=Thu Dec 7 14:53:36 2017 -0500
		io.openshift.build.commit.id=bbb670185b6ce67294cc461ae9c18710e6f26089
		io.openshift.build.commit.message=Merge pull request #18 from durandom/master
		io.openshift.build.commit.ref=master
3. oc edit bc/ruby/ex
>>>>>>>>>>>>>>>>>
  output:
    imageLabels:
    - name: io.openshift.build.commit.message
      value: registry.mycompany.com
    to:
      kind: ImageStreamTag
      name: ruby-ex:latest
>>>>>>>>>>>>>>>>
4. oc start-build ruby-ex
5. oc describe images sha256:65a449484f0f2618d0a16dd662f86f8a2decd6d77117395f411a92227bd3253f | grep "io.openshift.build.commit"
		io.openshift.build.commit.author=Ben Parees <bparees.github.com>
		io.openshift.build.commit.date=Thu Dec 7 14:53:36 2017 -0500
		io.openshift.build.commit.id=bbb670185b6ce67294cc461ae9c18710e6f26089
		io.openshift.build.commit.message=registry.mycompany.com
		io.openshift.build.commit.ref=master

Comment 15 errata-xmlrpc 2018-07-24 04:30:19 UTC

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2018:2232