Bug 1808402: Add yaml editor for values.yaml in helm install form #4513

rohitkrai03 · 2020-02-26T18:59:52Z

Fixes - https://issues.redhat.com/browse/ODC-3066

This PR -

Adds a new formik field that wraps EditYAML component.
Updates EditYAML with some new props to configure editor so that it's suitable in a Formik form.
Uses /api/helm/chart endpoint to fetch chart details and dump chart values in the yaml editor and then uses that to send payload to helm installation API.

Steps to test this PR -

Run ./build-backend.sh on latest master to get the backend API endpoint changes in bridge.
Run export HELM_REPOSITORY_CONFIG="/tmp/repositories.yaml" before running ./bin/bridge.

Screenshots -

openshift-ci-robot · 2020-02-28T12:22:37Z

@rohitkrai03: This pull request references Bugzilla bug 1808402, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.5.0) matches configured target release for branch (4.5.0)
bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1808402: Add yaml editor for values.yaml in helm install form

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

rohitkrai03 · 2020-02-28T12:23:23Z

/cherry-pick release-4.4

openshift-cherrypick-robot · 2020-02-28T12:23:24Z

@rohitkrai03: once the present PR merges, I will cherry-pick it on top of release-4.4 in a new PR and assign it to you.

In response to this:

/cherry-pick release-4.4

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot · 2020-02-28T14:40:18Z

@rohitkrai03: This pull request references Bugzilla bug 1808402, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.5.0) matches configured target release for branch (4.5.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1808402: Add yaml editor for values.yaml in helm install form

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

sahil143

@rohitkrai03 Install button is enabled even when yaml and release name is empty

divyanshiGupta · 2020-02-28T14:57:18Z

@rohitkrai03 Install button is enabled even when yaml and release name is empty

I think its because he removed the !dirty check https://github.com/openshift/console/pull/4513/files#diff-f0de5f537d36f5bc5d39101ea3ea898fL31.

@rohitkrai03 ^^

sahil143

@rohitkrai03 Height of YAML editor increases after it's rendered and also every time install is clicked.

Too much whitespace between Yaml Editor and Submit/Cancel button

sahil143 · 2020-02-28T15:19:46Z

Also, there is a scrollbar on this page whereas all other Yaml pages throughout console don't have one, probably because Yaml editor itself has scrollbar for code.

rohitkrai03 · 2020-02-28T16:41:07Z

@rohitkrai03 Install button is enabled even when yaml and release name is empty

Fixed this. Now release name field will always be populated with default value and yaml editor will only be shown if there is values.yaml to show.

rohitkrai03 · 2020-02-28T16:41:30Z

@rohitkrai03 Height of YAML editor increases after it's rendered and also every time install is clicked.

Too much whitespace between Yaml Editor and Submit/Cancel button

Fixed.

rohitkrai03 · 2020-02-28T16:43:01Z

Also, there is a scrollbar on this page whereas all other Yaml pages throughout console don't have one, probably because Yaml editor itself has scrollbar for code.

Other yaml pages let the editor handle create, update or other actions. The hieght is controlled by yaml editor along with the action buttons inside the editor. Now we are using it as a form field instead of self contained page, this is why the action buttons are not fixed at the bottom similar to all other forms that we have.

rohitkrai03 · 2020-02-28T17:14:43Z

/assign @christianvogt

christianvogt · 2020-02-28T23:48:24Z

frontend/packages/console-shared/src/components/formik-fields/YAMLEditorField.scss

@@ -0,0 +1,4 @@
+.yaml-editor-formik-field {


Suggested change

.yaml-editor-formik-field {

.ocs-yaml-editor-field {

christianvogt · 2020-02-29T00:31:14Z

frontend/packages/dev-console/src/components/helm/HelmInstallPage.tsx


  const initialValues: HelmInstallFormData = {
-    releaseName: '',
+    releaseName: chartName ? `${chartName}-defaulted` : '',


I know the screenshot in design has a suffix -defaulted but was this really the intention? Why not default to simply the chart name itself? Was this confirmed with UX or assumed based on screenshot?

cc @Veethika @serenamarie125

I just followed what the UX design doc had in the screenshot assuming that's what the ask was. @serenamarie125 @Veethika can you confirm?

Since it wasn't explicitly called out, i feel like it was just an artifact of design filling in arbitrary data.

I don't know how a suffix of -defaulted is better than simply the chart name itself.

Updated to have just the chart name as default pre-populated release name.

christianvogt · 2020-02-29T00:41:29Z

frontend/packages/console-shared/src/components/formik-fields/field-types.ts

+export interface YAMLEditorFieldProps extends FieldProps {
+  onChange?: (value: string) => void;
+}


Why do these prop types keep going into this file instead of co-located in the component file itself?

Some of the components such as DropdownField and NSDropdownField share prop types along with extending the base FieldProps. That is why all the field props are there in one file.

openshift-ci-robot · 2020-02-29T11:56:22Z

@rohitkrai03: This pull request references Bugzilla bug 1808402, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.5.0) matches configured target release for branch (4.5.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1808402: Add yaml editor for values.yaml in helm install form

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

rohitkrai03 · 2020-03-03T13:24:40Z

@serenamarie125 This PR - #4507 adds the ability in topology for a helm release to be added in an application group. But according to you we are not allowing that in 4.4. cc: @christianvogt

rohitkrai03 · 2020-03-03T16:34:02Z

/test e2e-gcp-console

divyanshiGupta · 2020-03-03T16:34:07Z

/lgtm

rohitkrai03 · 2020-03-03T16:42:55Z

/test e2e-gcp-console

rohitkrai03 · 2020-03-03T17:46:23Z

/test e2e-gcp-console

openshift-bot · 2020-03-03T20:42:04Z

/retest

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

christianvogt · 2020-03-03T23:12:36Z

/lgtm cancel

@rohitkrai03 size of the main vendor bundle has increased big time.

serenamarie125 · 2020-03-04T03:21:19Z

Yup, i'm saying to hide it from the secondary masthead when showing the Install Chart form - we can see it in the animated gifs

rohitkrai03 · 2020-03-04T07:33:46Z

/lgtm cancel

@rohitkrai03 size of the main vendor bundle has increased big time.

@christianvogt Not sure why that would happen, I am using the yaml editor as an async component. The whole page is on async route. So how would that affect vendor bundle size?

rohitkrai03 · 2020-03-04T10:16:53Z

I ran analyze locally on my branch as well as master. Below are the results which seem to be exactly same -

YAML Branch -

Master Branch -

rohitkrai03 · 2020-03-04T10:17:38Z

/test e2e-gcp-console

rohitkrai03 · 2020-03-04T10:18:06Z

@christianvogt Seems to be a flake in the test. The job succeeded when I reran it.

rohitkrai03 · 2020-03-04T12:06:13Z

/test e2e-gcp-console

andrewballantyne · 2020-03-04T13:32:31Z

@christianvogt Seems to be a flake in the test. The job succeeded when I reran it.

@rohitkrai03 @christianvogt Yeah... this is unfortunately a known bundle flake. I did briefly express shock to @spadgett about this a while back on a PR... I am still quite confused how a process of bundling is not deterministic. You'd think the items to bundle are static; must be some race condition to bundling x before y... and one gets a better result through uglify or something.

I believe it was related to Pattenfly icons the last time - not sure if that's always the case. Maybe we should look into this for 4.5 and try to cut down on this flake? (it happens pretty frequently)

christianvogt · 2020-03-04T18:07:32Z

/approve
/lgtm

openshift-ci-robot · 2020-03-04T18:08:59Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: christianvogt, divyanshiGupta, rohitkrai03

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:

~~frontend/OWNERS~~ [christianvogt]

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

openshift-ci-robot · 2020-03-04T21:39:44Z

@rohitkrai03: All pull requests linked via external trackers have merged. Bugzilla bug 1808402 has been moved to the MODIFIED state.

In response to this:

Bug 1808402: Add yaml editor for values.yaml in helm install form

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-cherrypick-robot · 2020-03-04T21:40:00Z

@rohitkrai03: new pull request created: #4642

In response to this:

/cherry-pick release-4.4

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot requested review from dtaylor113 and invincibleJai February 26, 2020 19:06

rohitkrai03 force-pushed the helm-yaml-editor branch from 9261dd1 to dfb2506 Compare February 28, 2020 11:56

rohitkrai03 changed the title ~~[WIP] Add yaml editor for values.yaml in helm install form~~ Bug 1808402: Add yaml editor for values.yaml in helm install form Feb 28, 2020

openshift-ci-robot added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Feb 28, 2020

rohitkrai03 force-pushed the helm-yaml-editor branch 2 times, most recently from 982bdac to 3e9c73b Compare February 28, 2020 14:21

sahil143 reviewed Feb 28, 2020

View reviewed changes

sahil143 suggested changes Feb 28, 2020

View reviewed changes

rohitkrai03 force-pushed the helm-yaml-editor branch from 3e9c73b to 56a485b Compare February 28, 2020 16:22

openshift-ci-robot assigned christianvogt Feb 28, 2020

christianvogt reviewed Feb 29, 2020

View reviewed changes

rohitkrai03 force-pushed the helm-yaml-editor branch from 1150c78 to 667cf48 Compare February 29, 2020 11:53

rohitkrai03 force-pushed the helm-yaml-editor branch from 325ea07 to 24c6f42 Compare March 3, 2020 14:38

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Mar 3, 2020

Fix extra padding issues in form and form footer

24c6f42

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 3, 2020

openshift-ci-robot removed 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 Mar 3, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 4, 2020

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 4, 2020

openshift-merge-robot merged commit 75b76b9 into openshift:master Mar 4, 2020

openshift-cherrypick-robot mentioned this pull request Mar 4, 2020

[release-4.4] Bug 1808404: Add yaml editor for values.yaml in helm install form #4642

Merged

spadgett added this to the v4.5 milestone Mar 9, 2020

rohitkrai03 deleted the helm-yaml-editor branch March 30, 2020 15:19

Bug 1808402: Add yaml editor for values.yaml in helm install form #4513

Bug 1808402: Add yaml editor for values.yaml in helm install form #4513

Conversation

rohitkrai03 commented Feb 26, 2020 • edited

Fixes - https://issues.redhat.com/browse/ODC-3066

This PR -

Steps to test this PR -

openshift-ci-robot commented Feb 28, 2020

rohitkrai03 commented Feb 28, 2020

openshift-cherrypick-robot commented Feb 28, 2020

openshift-ci-robot commented Feb 28, 2020

sahil143 left a comment

Choose a reason for hiding this comment

divyanshiGupta commented Feb 28, 2020 • edited

sahil143 left a comment

Choose a reason for hiding this comment

sahil143 commented Feb 28, 2020

rohitkrai03 commented Feb 28, 2020

rohitkrai03 commented Feb 28, 2020

rohitkrai03 commented Feb 28, 2020 • edited

rohitkrai03 commented Feb 28, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-ci-robot commented Feb 29, 2020

rohitkrai03 commented Mar 3, 2020

rohitkrai03 commented Mar 3, 2020

divyanshiGupta commented Mar 3, 2020

rohitkrai03 commented Mar 3, 2020

rohitkrai03 commented Mar 3, 2020

openshift-bot commented Mar 3, 2020

christianvogt commented Mar 3, 2020

serenamarie125 commented Mar 4, 2020

rohitkrai03 commented Mar 4, 2020 • edited

rohitkrai03 commented Mar 4, 2020

rohitkrai03 commented Mar 4, 2020

rohitkrai03 commented Mar 4, 2020 • edited

rohitkrai03 commented Mar 4, 2020

andrewballantyne commented Mar 4, 2020

christianvogt commented Mar 4, 2020

openshift-ci-robot commented Mar 4, 2020

openshift-ci-robot commented Mar 4, 2020

openshift-cherrypick-robot commented Mar 4, 2020

rohitkrai03 commented Feb 26, 2020 •

edited

divyanshiGupta commented Feb 28, 2020 •

edited

rohitkrai03 commented Feb 28, 2020 •

edited

rohitkrai03 commented Mar 4, 2020 •

edited

rohitkrai03 commented Mar 4, 2020 •

edited