Skip to content
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.

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

Helm Chart OpenShift Route Handling #238

Merged
merged 7 commits into from
May 17, 2024

Conversation

Jdubrick
Copy link
Contributor

@Jdubrick Jdubrick commented May 7, 2024

Please specify the area for this PR
Bug fix and small enhancement

What does does this PR do / why we need it:
This PR aims to address the issue that occurs when a user installs the devfile registry via Helm to an OpenShift environment. Prior to this change the fqdn of the registry-viewer was not properly being set as it was expecting global.ingress.domain to be set, but if you are installing to OpenShift it was not a requirement to set that.

In order to solve this problem the following was added:

  1. A helper script for OpenShift installations via Helm

    • This script allows a user to have the fqdn properly set if OpenShift auto generates a domain for you, or if you set a domain yourself. It utilizes Helm Upgrade to mimic the functionality currently in place for the registry operator where the fqdn is set after deployment (it spins up a new replacement pod after the information is known).
  2. Template updates

    • In order to have more clarity and consistency, the template used by Helm now has portions for both OpenShift Routes and Kubernetes Ingresses.
  3. Updated documentation

    • As a result of changes, documentation was updated to accurately reflect the different ways you can install the devfile registry with the use of a Helm chart.

Which issue(s) this PR fixes:

Fixes devfile/api#1467

PR acceptance criteria:

  • Test Coverage
    • Are your changes sufficiently tested, and are any applicable test cases added or updated to cover your changes?

Documentation (WIP)

How to test changes / Special notes to the reviewer:
The following cases will result in the fqdn of the registry-viewer being properly set:

  1. Install to OpenShift with the use of the new script WITH & WITHOUT setting a domain
  2. Install to OpenShift using the Helm CLI WITHOUT setting a domain
  3. Install to Kubernetes using the Helm CLI WITH & WITHOUT setting a domain

The following case will result in the fqdn not being properly set:

  1. Install to OpenShift with the Helm CLI WITH setting a domain

Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>
Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>
Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>
Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>
Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 7, 2024
Copy link
Member

@johnmcollier johnmcollier left a comment

Choose a reason for hiding this comment

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

Just a couple small comments

deploy/chart/devfile-registry/README.md Outdated Show resolved Hide resolved
deploy/chart/devfile-registry/README.md Show resolved Hide resolved
helm-openshift-install.sh Outdated Show resolved Hide resolved
Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>
Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 17, 2024
Copy link

openshift-ci bot commented May 17, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Jdubrick, johnmcollier

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

The pull request process is described here

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

@Jdubrick Jdubrick merged commit b1ec2c0 into devfile:main May 17, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.
Projects
None yet
2 participants