Show environment variables coming from the builder image in the Environment tab #889

jwforres · 2016-11-17T20:59:19Z

Fixes openshift/origin#11388
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1357107

jwforres · 2016-11-17T20:59:37Z

jwforres · 2016-11-17T21:04:11Z

Implemented for BCs but it is only supported for ImageStreamTag or ImageStreamImage. I hesitate to support this for DockerImage because it is going to put extra load onto the image import API. I didn't do this for individual builds for the same reason because they have already resolved to a particular DockerImage reference.

@bparees @spadgett PTAL

bparees · 2016-11-17T21:07:37Z

pretty.

jwforres · 2016-11-17T21:14:22Z

We could do the same thing for DCs that have an ICT

spadgett · 2016-11-17T22:31:08Z

app/scripts/controllers/buildConfig.js

+        if (_.includes(["ImageStreamTag", "ImageStreamImage"], buildFrom.kind)) {
+          lastLoadedBuildFromImageKey = buildFromImageKey;
+          DataService.get(APIService.kindToResource(buildFrom.kind), buildFrom.name, {namespace: buildFrom.namespace || $scope.projectName}).then(function(imageStreamImage){
+            $scope.BCEnvVarsFromImage = _.map(_.get(imageStreamImage, 'image.dockerImageMetadata.Config.Env'),


Should be able to use $filter('imageEnv')

https://github.com/spadgett/origin-web-console/blob/ca4be237df6935819e795eeb2962e985230876e1/app/scripts/filters/resources.js#L287-L298

Nevermind, that's looking for a specific key. We might generalize it though or add another filter

spadgett · 2016-11-17T22:32:17Z

app/views/browse/build-config.html

@@ -365,6 +365,21 @@
                  <uib-tab heading="Environment" active="selectedTab.environment" ng-if="buildConfig && !(buildConfig | isJenkinsPipelineStrategy)">
                    <uib-tab-heading>Environment</uib-tab-heading>
                    <h3>Environment Variables</h3>
+                    <div style="margin-top: -5px;" ng-if="BCEnvVarsFromImage.length">
+                      The builder image has additional environment variables defined.  Variables defined below will overwrite any from the image with the same name.
+                      <a href="" ng-click="expand.imageEnv = true" ng-if="!expand.imageEnv">View image environment variables</a>


"Show image environment variables"? To be consistent with other show/hide links

spadgett · 2016-11-17T22:33:12Z

app/views/browse/build-config.html

@@ -365,6 +365,21 @@
                  <uib-tab heading="Environment" active="selectedTab.environment" ng-if="buildConfig && !(buildConfig | isJenkinsPipelineStrategy)">
                    <uib-tab-heading>Environment</uib-tab-heading>
                    <h3>Environment Variables</h3>
+                    <div style="margin-top: -5px;" ng-if="BCEnvVarsFromImage.length">


class="mar-top-sm"?

its negative margin, i dont think we have a class for applying negative margin?

You're right

spadgett · 2016-11-17T22:40:05Z

We could do the same thing for DCs that have an ICT

I think we have the image stream image already to fill in the pod template details.

jwforres · 2016-11-18T16:38:52Z

@spadgett as discussed this will just cover the BC env vars, will work on another PR for DCs

spadgett · 2016-11-18T18:54:05Z

app/scripts/services/images.js

+                var pair = entry.split('=');
+                return {
+                  name: _.head(pair),
+                  value:_.last(pair)


Not a new problem, but this won't correctly handle env var values with a =. Maybe

var index = entry.indexOf('='); return { name: entry.substring(0, index), value: entry.substring(index + 1) };

jwforres · 2016-11-18T19:48:31Z

Yes we have an open / bug issue for that. I can look at fixing as part of
this

On Nov 18, 2016 1:54 PM, "Sam Padgett" [email protected] wrote:

@spadgett commented on this pull request.

In app/scripts/services/images.js
#889 (review)
:

@@ -213,10 +213,23 @@ angular.module("openshiftConsole")
return resources;
};
var getEnvironment = function(imageStreamImage) {
 return _.map(_.get(imageStreamImage, 'image.dockerImageMetadata.Config.Env'),
         function(entry) {
           var pair = entry.split('=');
           return {
             name: _.head(pair),
             value:_.last(pair)
Not a new problem, but this won't correctly handle env var values with a =.
Maybe

var index = entry.indexOf('=');return {
name: entry.substring(0, index),
value: entry.substring(index + 1)
};

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#889 (review),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABZk7dShY_zT4qez7Ql5uEcLLUVYK-S1ks5q_fRNgaJpZM4K1zXQ
.

…onment tab Fixes openshift/origin#11388

jwforres · 2016-11-21T16:35:43Z

@spadgett updated to handle env var values with =

spadgett · 2016-11-21T16:38:38Z

[merge]

openshift-bot · 2016-11-21T16:39:28Z

Evaluated for origin web console merge up to 24e766a

openshift-bot · 2016-11-21T16:39:28Z

[Test]ing while waiting on the merge queue

openshift-bot · 2016-11-21T16:47:20Z

Evaluated for origin web console test up to 24e766a

openshift-bot · 2016-11-21T17:04:12Z

Origin Web Console Test Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_web_console/737/) (Base Commit: 9686826)

openshift-bot · 2016-11-21T17:39:14Z

Origin Web Console Merge Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_web_console/740/) (Base Commit: 07653ad)

spadgett reviewed Nov 17, 2016

View reviewed changes

jwforres force-pushed the bc-image-env branch from 02d266d to 73bec0c Compare November 18, 2016 16:38

jwforres changed the title ~~[WIP] Show environment variables coming from the builder image in the Environment tab~~ Show environment variables coming from the builder image in the Environment tab Nov 18, 2016

spadgett reviewed Nov 18, 2016

View reviewed changes

spadgett self-assigned this Nov 21, 2016

Show environment variables coming from the builder image in the Envir…

24e766a

…onment tab Fixes openshift/origin#11388

jwforres force-pushed the bc-image-env branch from 73bec0c to 24e766a Compare November 21, 2016 16:35

spadgett approved these changes Nov 21, 2016

View reviewed changes

openshift-bot merged commit 00e62d0 into openshift:master Nov 21, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show environment variables coming from the builder image in the Environment tab #889

Show environment variables coming from the builder image in the Environment tab #889

jwforres commented Nov 17, 2016 •

edited

Loading

jwforres commented Nov 17, 2016

jwforres commented Nov 17, 2016

bparees commented Nov 17, 2016

jwforres commented Nov 17, 2016

spadgett Nov 17, 2016

spadgett Nov 17, 2016

spadgett Nov 17, 2016

spadgett Nov 17, 2016

jwforres Nov 18, 2016

spadgett Nov 18, 2016

spadgett commented Nov 17, 2016

jwforres commented Nov 18, 2016

spadgett Nov 18, 2016

jwforres commented Nov 18, 2016

@spadgett commented on this pull request.

jwforres commented Nov 21, 2016

spadgett commented Nov 21, 2016

openshift-bot commented Nov 21, 2016

openshift-bot commented Nov 21, 2016

openshift-bot commented Nov 21, 2016

openshift-bot commented Nov 21, 2016

openshift-bot commented Nov 21, 2016 •

edited

Loading

Show environment variables coming from the builder image in the Environment tab #889

Show environment variables coming from the builder image in the Environment tab #889

Conversation

jwforres commented Nov 17, 2016 • edited Loading

jwforres commented Nov 17, 2016

jwforres commented Nov 17, 2016

bparees commented Nov 17, 2016

jwforres commented Nov 17, 2016

spadgett Nov 17, 2016

Choose a reason for hiding this comment

spadgett Nov 17, 2016

Choose a reason for hiding this comment

spadgett Nov 17, 2016

Choose a reason for hiding this comment

spadgett Nov 17, 2016

Choose a reason for hiding this comment

jwforres Nov 18, 2016

Choose a reason for hiding this comment

spadgett Nov 18, 2016

Choose a reason for hiding this comment

spadgett commented Nov 17, 2016

jwforres commented Nov 18, 2016

spadgett Nov 18, 2016

Choose a reason for hiding this comment

jwforres commented Nov 18, 2016

@spadgett commented on this pull request.

jwforres commented Nov 21, 2016

spadgett commented Nov 21, 2016

openshift-bot commented Nov 21, 2016

openshift-bot commented Nov 21, 2016

openshift-bot commented Nov 21, 2016

openshift-bot commented Nov 21, 2016

openshift-bot commented Nov 21, 2016 • edited Loading

jwforres commented Nov 17, 2016 •

edited

Loading

openshift-bot commented Nov 21, 2016 •

edited

Loading