Add Goflag option to define branding at build time #126

zherman0 · 2019-02-05T20:54:42Z

Add Goflag option to define branding at build time. Docker rhel file has been updated to provide ocp branding.
Usage: ADDITIONAL_GOTAGS="ocp" make WHAT="cmd/console"
$ console version
ConsoleOperator v0.0.1
Git Commit: 6cf4377
Build Date: 2019-02-05T20:08:03Z
Current Brand Setting: ocp
Tag option: ocp
This solution uses tags in the build process.

Additionally, updated the console version helper tool to show current Git Commit value, build date, and the brand setting.

zherman0 · 2019-02-05T21:16:32Z

@spadgett @benjaminapetersen - Created a build time solution using the goflags var and using tags.

Dockerfile

zherman0 · 2019-02-05T23:51:36Z

@spadgett @benjaminapetersen - Simplified the flag situation to only have one -ocp flag. The docker rhel file uses that flag otherwise everything defaults to okd.

spadgett

@benjaminapetersen do you know how these scripts get updated in the different repos? wondering if we will lose the change here if we ever update the hack scripts

hack/lib/build/binaries.sh

openshift-bot · 2019-02-06T06:21:11Z

/retest

pkg/cmd/version/cmd.go

zherman0 · 2019-02-12T16:41:07Z

/test e2e-aws

Dockerfile.rhel7

benjaminapetersen · 2019-02-12T19:51:19Z

pkg/console/subresource/configmap/brand_ocp.go

+package configmap
+
+const (
+	BRAND = "ocp"


for ref, this BRAND will be the fallback for getBrand() from:
https://github.com/openshift/console-operator/pull/134/files#diff-bc08d2d514ac5156db922f45487af976R46

Are you sure this is resolved? I would have expected to see an update to configmap.go in this PR.

benjaminapetersen · 2019-02-12T19:51:35Z

pkg/console/subresource/configmap/brand_okd.go

+package configmap
+
+const (
+	BRAND = "okd"


Same comment

benjaminapetersen · 2019-02-13T20:14:27Z

/lgtm

zherman0 · 2019-02-13T21:50:45Z

/test e2e-aws

zherman0 · 2019-02-13T22:56:24Z

/test e2e-aws

zherman0 · 2019-02-14T00:31:01Z

flake
/test e2e-aws

benjaminapetersen · 2019-02-14T01:33:02Z

/retest

benjaminapetersen · 2019-02-14T02:24:18Z

@zherman0 this will need a rebase. There is a small conflict now, its likely just updating the getBrand() func, to return your new BRAND const instead of defaultBrand.

benjaminapetersen · 2019-02-14T14:50:32Z

pkg/console/subresource/configmap/brand_ocp.go

+package configmap
+
+const (
+	BRAND = "ocp"


Got me curious about the default docs URL as well... if that should be toggled for ocp vs okd. Currently in configmap.go its just "https://docs.okd.io/4.0/".

@ahardin-rh helped dig up this alternative URL: https://docs.openshift.com/container-platform/4.0/welcome/index.html

I don't know yet if we should use or not (it is also password protected). Fishing around for this.

@zherman0 yes, I believe this is the correct URL to use. It is password protected for now as it is under active dev. It will not be later.

zherman0 · 2019-02-14T17:23:10Z

/retest

benjaminapetersen · 2019-02-14T17:33:08Z

/lgtm

openshift-ci-robot · 2019-02-14T17:33:23Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: benjaminapetersen, zherman0

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [benjaminapetersen]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

benjaminapetersen · 2019-02-14T18:40:30Z

bam, merged.

openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 5, 2019

openshift-ci-robot requested review from benjaminapetersen and enj February 5, 2019 20:54

zherman0 force-pushed the goflags branch from 7f089e6 to 6365f75 Compare February 5, 2019 21:01

openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 5, 2019

zherman0 force-pushed the goflags branch from 6365f75 to 536907b Compare February 5, 2019 21:13

openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Feb 5, 2019

spadgett reviewed Feb 5, 2019

View reviewed changes

Dockerfile Outdated Show resolved Hide resolved

spadgett reviewed Feb 6, 2019

View reviewed changes

hack/lib/build/binaries.sh Outdated Show resolved Hide resolved

benjaminapetersen reviewed Feb 6, 2019

View reviewed changes

pkg/cmd/version/cmd.go Show resolved Hide resolved

benjaminapetersen mentioned this pull request Feb 12, 2019

Honor configs & report public hostname on status #134

Merged

zherman0 force-pushed the goflags branch from 953cf1b to bd41067 Compare February 12, 2019 16:09

benjaminapetersen reviewed Feb 12, 2019

View reviewed changes

Dockerfile.rhel7 Show resolved Hide resolved

benjaminapetersen reviewed Feb 12, 2019

View reviewed changes

pkg/console/subresource/configmap/brand_okd.go Outdated

package configmap

const (

BRAND = "okd"

Copy link

Contributor

benjaminapetersen Feb 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment

openshift-ci-robot assigned benjaminapetersen Feb 13, 2019

openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Feb 13, 2019

zherman0 force-pushed the goflags branch from bd41067 to 5aa0a2a Compare February 14, 2019 03:25

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Feb 14, 2019

benjaminapetersen reviewed Feb 14, 2019

View reviewed changes

Goflag branding

5a7ddca

zherman0 force-pushed the goflags branch from 1249f36 to 5a7ddca Compare February 14, 2019 17:31

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 14, 2019

openshift-merge-robot merged commit 636eb08 into openshift:master Feb 14, 2019

Add Goflag option to define branding at build time #126

Add Goflag option to define branding at build time #126

Uh oh!

Conversation

zherman0 commented Feb 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zherman0 commented Feb 5, 2019

Uh oh!

Uh oh!

zherman0 commented Feb 5, 2019

Uh oh!

spadgett left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

openshift-bot commented Feb 6, 2019

Uh oh!

Uh oh!

zherman0 commented Feb 12, 2019

Uh oh!

Uh oh!

benjaminapetersen Feb 12, 2019

Choose a reason for hiding this comment

Uh oh!

benjaminapetersen Feb 14, 2019

Choose a reason for hiding this comment

Uh oh!

benjaminapetersen Feb 12, 2019

Choose a reason for hiding this comment

Uh oh!

benjaminapetersen commented Feb 13, 2019

Uh oh!

zherman0 commented Feb 13, 2019

Uh oh!

zherman0 commented Feb 13, 2019

Uh oh!

zherman0 commented Feb 14, 2019

Uh oh!

benjaminapetersen commented Feb 14, 2019

Uh oh!

benjaminapetersen commented Feb 14, 2019

Uh oh!

benjaminapetersen Feb 14, 2019

Choose a reason for hiding this comment

Uh oh!

benjaminapetersen Feb 14, 2019

Choose a reason for hiding this comment

Uh oh!

benjaminapetersen Feb 14, 2019

Choose a reason for hiding this comment

Uh oh!

zherman0 commented Feb 14, 2019

Uh oh!

benjaminapetersen commented Feb 14, 2019

Uh oh!

openshift-ci-robot commented Feb 14, 2019

Uh oh!

benjaminapetersen commented Feb 14, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

zherman0 commented Feb 5, 2019 •

edited

Loading