Skip to content

Custom brand revisions #245

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
openshift-merge-robot merged 5 commits into from
Jun 24, 2019
Merged

Custom brand revisions #245

openshift-merge-robot merged 5 commits into from
Jun 24, 2019

Conversation

@benjaminapetersen
Copy link
Contributor

@benjaminapetersen benjaminapetersen commented Jun 21, 2019

Feature:

  • Provide a Custom Product Name and Custom Logo to be displayed in the console via operator configuration.

Requires additional RBAC to run:

  • Pull the branch & have a dev cluster handy
  • Disable CVO so you can deploy your custom operator image
    oc scale deployment cluster-version-operator --replicas 0 --namespace openshift-cluster-version
  • Docker build docker build -t quay.io/<you>/console-operator:latest .
  • Docker push docker push quay.io/<you>/console-operator:latest
  • Apply the additional necessary Rolebinding oc apply -f manifests/04-rbac-rolebinding.yaml
  • Apply the additional Role oc apply -f manifests/03-rbac-role-ns-openshift-config.yaml
  • Deploy your custom operator image (example template) (- "-v=4" or higher recommended for logging)

To use:

  • Create a ConfigMap with a logo image in the openshift-config namespace.

If an image is needed, this fits the approximate dimensions:

fake-logo

oc create configmap my-custom-logo-file --from-file ~/Desktop/my-logo.png
  • Update the operator config to reference the logo created:
oc apply -f <this-file>
apiVersion: operator.openshift.io/v1
kind: Console
metadata:
  name: cluster
spec:
  customization:
    customProductName: my-awesome-cluster
    customLogoFile:
      name: my-custom-logo-file
      key: my-logo.png

Expected function:

  • A resource syncer is scanning openshift-config for configmap changes, but does nothing with observed configmaps unless specified via the operator config.
  • If a customLogoFile.name exists in the operator config, the resource syncer will sync this configmap into openshift-console as a new configmap called custom-logo.
  • The operator main sync loop will notice when the new custom-logo configmap appears in the openshift-console namespace and will trigger a sync of console resources, which will cause a rollout of the deployment with the logo file mounted via a volume mount.
  • if the source configmap is deleted, the custom logo file will be deleted
  • if the custom logo is removed from the operator-config, the volume mount will be deleted

/assign @jhadvig
@spadgett fyi

@openshift-ci-robot openshift-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 21, 2019
zherman0 and others added 2 commits June 21, 2019 00:42
@zherman0 @benjaminapetersen
Add custom branding to config map
09f140f
@enj @benjaminapetersen
Add secrets wrapper
39ba852 
Signed-off-by: Monis Khan <mkhan@redhat.com>
benjaminapetersen
benjaminapetersen commented Jun 21, 2019
View reviewed changes
benjaminapetersen
benjaminapetersen commented Jun 21, 2019
View reviewed changes
benjaminapetersen
benjaminapetersen commented Jun 21, 2019
View reviewed changes
benjaminapetersen
benjaminapetersen commented Jun 21, 2019
View reviewed changes
benjaminapetersen
benjaminapetersen commented Jun 21, 2019
View reviewed changes
enj
enj reviewed Jun 21, 2019
View reviewed changes
Copy link
Contributor

@enj enj left a comment

Choose a reason for hiding this comment

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

Non-comprehensive half asleep review.

jhadvig
jhadvig reviewed Jun 21, 2019
View reviewed changes
Copy link
Member

@jhadvig jhadvig left a comment

Choose a reason for hiding this comment

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

A lot of work done, nice !
Adding a few comments to start the merge process 😄

Noticed that the new TestOperatorConfigCustomization is failing.

=== RUN   TestOperatorConfigCustomization
--- FAIL: TestOperatorConfigCustomization (40.22s)
    brand_customization_test.go:153: productName: test-e2e-product-name, logo: /var/logo/pic.jpg
    brand_customization_test.go:91: error: customization values not on deployment, timed out waiting for the condition

@benjaminapetersen benjaminapetersen force-pushed the custom-brand-revisions branch 2 times, most recently from 4b723ed to 1e9744b Compare June 21, 2019 14:06
@benjaminapetersen
Copy link
Contributor Author

benjaminapetersen commented Jun 21, 2019 •
edited
Loading

The following cases show how the resourceSyncer provides additional status information to us based on a variety of use cases:

  • Incorrectly set
customization:
   customLogoFile:
    key: fake-logo.png
    # no name provided

renders out the following conditions, sync is idle:

  conditions:
  - lastTransitionTime: "2019-06-19T20:50:33Z"
    status: "False"
    type: ResourceSyncControllerDegraded
  - lastTransitionTime: "2019-06-19T20:50:33Z"
    reason: NoUnsupportedConfigOverrides
    status: "True"
    type: UnsupportedConfigOverridesUpgradeable
  - lastTransitionTime: "2019-06-21T15:46:09Z"
    message: custom logo file name and key must both be set
    reason: SynchronizationError
    status: "True"
    type: Degraded
  - lastTransitionTime: "2019-06-21T15:45:56Z"
    message: custom logo file name and key must both be set
    reason: SynchronizationError
    status: "True"
    type: Progressing
  - lastTransitionTime: "2019-06-20T18:42:09Z"
    message: 2 replicas ready at version 0.0.1-snapshot
    status: "True"
    type: Available
  • setting
customization:
   customLogoFile:
    # no key provided
    name: fake-logo

renders the following conditions, sync is idle:

status:
  conditions:
  - lastTransitionTime: "2019-06-19T20:50:33Z"
    status: "False"
    type: ResourceSyncControllerDegraded
  - lastTransitionTime: "2019-06-19T20:50:33Z"
    reason: NoUnsupportedConfigOverrides
    status: "True"
    type: UnsupportedConfigOverridesUpgradeable
  - lastTransitionTime: "2019-06-21T15:48:13Z"
    message: custom logo file name and key must both be set
    reason: SynchronizationError
    status: "True"
    type: Degraded
  - lastTransitionTime: "2019-06-21T15:45:56Z"
    message: custom logo file name and key must both be set
    reason: SynchronizationError
    status: "True"
    type: Progressing
  - lastTransitionTime: "2019-06-20T18:42:09Z"
    message: 2 replicas ready at version 0.0.1-snapshot
    status: "True"
    type: Available

providing valid config, but without creating a configmap:

# valid, but fake-logo configmap does not yet exist 
  customization:
    customLogoFile:
      name: fake-logo
      key: fake-logo.png

renders the following conditions, but sync is in a loop:

status:
  conditions:
  - lastTransitionTime: "2019-06-19T20:50:33Z"
    status: "False"
    type: ResourceSyncControllerDegraded
  - lastTransitionTime: "2019-06-19T20:50:33Z"
    reason: NoUnsupportedConfigOverrides
    status: "True"
    type: UnsupportedConfigOverridesUpgradeable
  - lastTransitionTime: "2019-06-21T15:51:13Z"
    message: custom logo file fake-logo not found
    reason: SynchronizationError
    status: "True"
    type: Degraded
  - lastTransitionTime: "2019-06-21T15:45:56Z"
    message: custom logo file fake-logo not found
    reason: SynchronizationError
    status: "True"
    type: Progressing
  - lastTransitionTime: "2019-06-20T18:42:09Z"
    message: 2 replicas ready at version 0.0.1-snapshot
    status: "True"
    type: Available

log error in this case is E0621 15:52:12.315056 1 controller.go:129] {Console Console} failed with: custom logo file fake-logo not found

Providing the following and creating fake-logo via the command oc create configmap fake-logo --from-file ~/Desktop/imgs/fake-logo.png -n openshift-config:

  customization:
    customProductName: Muh Clusta
    customLogoFile:
      name: fake-logo
      key: fake-logo.png

yields the following, sync is settled:

status:
  conditions:
  - lastTransitionTime: "2019-06-19T20:50:33Z"
    status: "False"
    type: ResourceSyncControllerDegraded
  - lastTransitionTime: "2019-06-19T20:50:33Z"
    reason: NoUnsupportedConfigOverrides
    status: "True"
    type: UnsupportedConfigOverridesUpgradeable
  - lastTransitionTime: "2019-06-21T15:53:10Z"
    status: "False"
    type: Degraded
  - lastTransitionTime: "2019-06-21T15:45:56Z"
    message: Working toward version 0.0.1-snapshot
    reason: SyncLoopProgressing
    status: "True"
    type: Progressing
  - lastTransitionTime: "2019-06-20T18:42:09Z"
    message: 2 replicas ready
    status: "True"
    type: Available
  • removing the customization.customLogoFile removes the configmap, sync is idle.

benjaminapetersen
benjaminapetersen commented Jun 21, 2019
View reviewed changes
@benjaminapetersen benjaminapetersen force-pushed the custom-brand-revisions branch 2 times, most recently from 29e091f to f80d913 Compare June 21, 2019 17:13
@benjaminapetersen benjaminapetersen mentioned this pull request Jun 21, 2019
Closed
@benjaminapetersen benjaminapetersen force-pushed the custom-brand-revisions branch 2 times, most recently from 944bdc5 to f45852a Compare June 24, 2019 05:38
benjaminapetersen
benjaminapetersen commented Jun 24, 2019
View reviewed changes
@jhadvig
Copy link
Member

jhadvig commented Jun 24, 2019

/retest

@jhadvig
Copy link
Member

jhadvig commented Jun 24, 2019

probably a flake

=== RUN   TestOperatorConfigBranding
--- FAIL: TestOperatorConfigBranding (37.42s)
    branding_test.go:69: update operator with okd
    branding_test.go:69: update operator with ocp
    branding_test.go:69: update operator with online
    branding_test.go:69: update operator with dedicated
    branding_test.go:69: update operator with azure

/retest

jhadvig
jhadvig approved these changes Jun 24, 2019
View reviewed changes
Copy link
Member

@jhadvig jhadvig left a comment

Choose a reason for hiding this comment

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

Just couple of small nits, that can be addressed afterwards :)

@jhadvig
Copy link
Member

jhadvig commented Jun 24, 2019

/lgtm
/retest

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 24, 2019
jhadvig and others added 2 commits June 24, 2019 17:25
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jun 24, 2019
@jhadvig
Copy link
Member

jhadvig commented Jun 24, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 24, 2019
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jun 24, 2019

[APPROVALNOTIFIER] This PR is APPROVED

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

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:

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

@openshift-bot
Copy link
Contributor

openshift-bot commented Jun 24, 2019

/retest

Please review the full test history for this PR and help us cut down flakes.

@benjaminapetersen
Copy link
Contributor Author

benjaminapetersen commented Jun 24, 2019

/retest

@openshift-merge-robot openshift-merge-robot merged commit cb3352e into openshift:master Jun 24, 2019
@benjaminapetersen benjaminapetersen deleted the custom-brand-revisions branch June 25, 2019 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jhadvig jhadvig jhadvig approved these changes

@spadgett spadgett Awaiting requested review from spadgett

+1 more reviewer

@enj enj enj left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@jhadvig jhadvig

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@benjaminapetersen @jhadvig @openshift-ci-robot @openshift-bot @enj @openshift-merge-robot @zherman0