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

Bug 1834186: Change Input of Independent Mode to Text Area from Text Input #5241

Merged
merged 1 commit into from May 14, 2020

Conversation

bipuladh
Copy link
Contributor

@bipuladh bipuladh commented Apr 30, 2020

Screenshots
Before File Upload:
Screenshot from 2020-05-07 10-28-57
After File Upload:
Screenshot from 2020-05-07 10-28-26

UXD
https://marvelapp.com/da7ai67/screen/65092932

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 30, 2020
@openshift-ci-robot openshift-ci-robot added component/ceph Related to ceph-storage-plugin approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Apr 30, 2020
@bipuladh bipuladh changed the title [WIP] Change Input of Indepent Mode to Text Area from Text Input Change Input of Indepent Mode to Text Area from Text Input May 6, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 6, 2020
@bipuladh bipuladh changed the title Change Input of Indepent Mode to Text Area from Text Input Change Input of Independent Mode to Text Area from Text Input May 6, 2020
@gnehapk
Copy link
Contributor

gnehapk commented May 7, 2020

@bipuladh Can you please add the screenshot of the view once the json file is uploaded. Wants to see how it looks after uploading.

@gnehapk
Copy link
Contributor

gnehapk commented May 7, 2020

@@ -143,40 +99,15 @@ const InstallExternalCluster = withHandlePromise((props: InstallExternalClusterP
.catch((e) => {
// eslint-disable-next-line no-console
console.error(e);
// Remove secret if cluster creation was not possible
handlePromise(k8sKill(SecretModel, secret));
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@@ -30,6 +59,11 @@ export const checkForIndependentSupport = (csv: ClusterServiceVersionKind): bool
return independent === 'true';
};

export const getRequiredKeys = (csv: ClusterServiceVersionKind): string[] => {
const keys = csv.metadata.annotations?.['external.cluster.ocs.openshift.io/keys'];
return keys?.split(',');
Copy link
Contributor

Choose a reason for hiding this comment

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

are you expecting keys to be a string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

similar to the feature annotation so yes.

Copy link
Contributor

Choose a reason for hiding this comment

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

sounds good!

@@ -30,6 +59,11 @@ export const checkForIndependentSupport = (csv: ClusterServiceVersionKind): bool
return independent === 'true';
};

export const getRequiredKeys = (csv: ClusterServiceVersionKind): string[] => {
const keys = csv.metadata.annotations?.['external.cluster.ocs.openshift.io/keys'];
return keys?.split(',');
Copy link
Contributor

Choose a reason for hiding this comment

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

sounds good!

@cloudbehl
Copy link
Contributor

You haven't added this to the independent mode request?

  crashCollector:
    disable: true

@bipuladh
Copy link
Contributor Author

bipuladh commented May 8, 2020

You haven't added this to the independent mode request?

  crashCollector:
    disable: true

I don't see it used anywhere else. Is it documented anywhere?

@cloudbehl
Copy link
Contributor

Please check the document by Sebastian.

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 9, 2020
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 11, 2020
@bipuladh bipuladh requested a review from cloudbehl May 11, 2020 04:31
@cloudbehl
Copy link
Contributor

@bipuladh add bugzilla!

@bipuladh bipuladh force-pushed the indep_mode_design branch 2 times, most recently from 4c9019e to 3d6ef49 Compare May 11, 2020 09:09
@bipuladh bipuladh changed the title Change Input of Independent Mode to Text Area from Text Input Bug 1834186: Change Input of Independent Mode to Text Area from Text Input May 11, 2020
Copy link
Contributor

@cloudbehl cloudbehl left a comment

Choose a reason for hiding this comment

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

NIT

/>
</div>
</div>
)}
{(isIndependent === false || mode === MODES.CONVERGED) && (
<CreateOCSServiceForm match={match} />
)}
{mode === MODES.INDEPENDENT && <InstallExternalCluster match={match} />}
{mode === MODES.INDEPENDENT && independentReqdKeys && downloadFile && (
Copy link
Contributor

Choose a reason for hiding this comment

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

downloadFile should not be a hard requirement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not doing this is a source of bug.

@a2batic
Copy link
Contributor

a2batic commented May 12, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label May 12, 2020
@bipuladh
Copy link
Contributor Author

/hold needs to be in sync with red-hat-storage/ocs-operator#499

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 12, 2020
@bipuladh
Copy link
Contributor Author

/bugzilla refresh

@openshift-ci-robot openshift-ci-robot added bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. and removed bugzilla/severity-unspecified Referenced Bugzilla bug's severity is unspecified for the PR. labels May 12, 2020
@openshift-ci-robot
Copy link
Contributor

@bipuladh: This pull request references Bugzilla bug 1834186, 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:

/bugzilla refresh

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 openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label May 13, 2020
@bipuladh
Copy link
Contributor Author

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 14, 2020
@cloudbehl
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label May 14, 2020
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: a2batic, bipuladh, cloudbehl

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

@openshift-merge-robot openshift-merge-robot merged commit 4a1b0e4 into openshift:master May 14, 2020
@openshift-ci-robot
Copy link
Contributor

@bipuladh: All pull requests linked via external trackers have merged: openshift/console#5241. Bugzilla bug 1834186 has been moved to the MODIFIED state.

In response to this:

Bug 1834186: Change Input of Independent Mode to Text Area from Text Input

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.

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. bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. component/ceph Related to ceph-storage-plugin lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants