Add logic to apply ConsoleLink #268
Conversation
|
Skipping CI for Draft Pull Request. |
|
@zdtsw since this manifest file is only creating a route, I think our intent was to replace it immediately by an API call to create the route and avoid the manifest file. Thoughts? |
you mean to not have |
|
/hold |
zdtsw
changed the title
| @@ -91,11 +92,18 @@ func (d *Dashboard) ReconcileComponent(owner metav1.Object, cli client.Client, s | |||
| if err != nil { | |||
| return fmt.Errorf("failed to set dashboard ISV from %s", PathISVAddOn) | |||
| } | |||
| // ConsoleLink handling | |||
| err = deploy.DeployManifestsFromPath(owner, cli, ComponentName, | |||
There was a problem hiding this comment.
Wouldn't we want to get url from dashboard route and update the ConsoleLink yaml first https://github.com/red-hat-data-services/odh-manifests/blob/feature-rearchitecture/odh-dashboard/consolelink/consolelink.yaml#L9 before applying it ?
There was a problem hiding this comment.
strange, the other file of generating replacement is not in this PR. must get lost in the old branch.
i will update it
There was a problem hiding this comment.
not sure if this is the right way.
so i want to reuse ReplaceStringsInFile function to replace variable in manifests, and it is currently in dscinitialzation.utils, to avoid cycle import, i moved the function into pkg/common
in theory, these lines of get route, replace can be done in a function instead of inject in the function
There was a problem hiding this comment.
Moved the RelaceStrings to common, is this ready to merge ?
There was a problem hiding this comment.
yes, think so.
it is using common.ReplaceStringsInFile()
components/dashboard/dashboard.go
Outdated
| PathISVCommon = deploy.DefaultManifestPath + "/" + ComponentName + "/overlays/apps" | ||
| PathISVSM = deploy.DefaultManifestPath + "/" + ComponentName + "/overlays/apps-onpre" | ||
| PathISVAddOn = deploy.DefaultManifestPath + "/" + ComponentName + "/overlays/apps-addon" | ||
| ComponentName = "odh-dashboard" |
There was a problem hiding this comment.
@zdtsw I think you need to rebase?
|
/unhold |
- Move from util to common for ReplaceStringsInFile Signed-off-by: Wen Zhou <wenzhou@redhat.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: VaishnaviHire 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 |
zdtsw
merged commit 9ca4404
into
opendatahub-io:feature-rearchitecture
Description
Add logic to apply ConsoleLink when it is only for downstream cluster
Ref: red-hat-data-services/odh-manifests#426
#261
Depend on opendatahub-io/odh-manifests#891How Has This Been Tested?
Merge criteria: