Create OCS Service workflow #2359

gnehapk · 2019-08-15T06:02:23Z

Screenshots for the current implementation -

Planning to send a separate PR for the following pending items -

Pre-select the nodes if they are already labeled
Few issues - sorting, on selection the page is getting reloaded

@vojtechszocs @spadgett @priley86 Please review

priley86 · 2019-08-15T12:42:37Z

frontend/public/components/factory/table.tsx

@@ -230,13 +230,13 @@ const VirtualBody: React.SFC<VirtualBodyProps> = (props) => {
  );


thanks @gnehapk - updates in Table.tsx look correct to me per our last discussion. I think customData should be any type and can be used to pass selection state back to the view if desired. The event.preventDefault() is necessary since we are not handling this in PF React.

lizsurette · 2019-08-15T15:05:22Z

@yuvalgalanti @shirimordechay Would you guys be able to give this a review from a UXD standpoint?

umangachapagain · 2019-08-16T05:21:22Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/ocs-install.tsx

+            path: labelPath,
+          },
+        ];
+        patch[0].value = { ...patch[0].value, 'cluster.ocs.openshift.io/openshift-storage': '' };


can we make 'cluster.ocs.openshift.io/openshift-storage': '' as const nodeLabel?

umangachapagain · 2019-08-16T05:23:40Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/ocs-install.tsx

+          const taints = node.spec.taints
+            ? [
+                ...node.spec.taints,
+                { key: 'node.ocs.openshift.io/storage', value: 'true', effect: 'NoSchedule' },


Make 'node.ocs.openshift.io/storage' as const ocsTaint or something? Prevents repetition and helps making changes easier.

umangachapagain · 2019-08-16T05:23:53Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/ocs-install.tsx

+                ...node.spec.taints,
+                { key: 'node.ocs.openshift.io/storage', value: 'true', effect: 'NoSchedule' },
+              ]
+            : [{ key: 'node.ocs.openshift.io/storage', value: 'true', effect: 'NoSchedule' }];


same as before

umangachapagain · 2019-08-16T05:25:47Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/ocs-install.tsx

+        }),
+    );
+
+    const obj: K8sResourceKind = {


Can we keep this as a template in a different file? Having YAML definition in between UI logic looks messy to me.

shirimordechay · 2019-08-18T08:04:05Z

frontend/public/components/utils/request-size-input.tsx

@@ -33,6 +33,7 @@ export class RequestSizeInput extends React.Component<RequestSizeInputProps> {
            aria-describedby={describedBy}
            name={inputName}
            required={this.props.required}
+            value={this.props.defaultRequestSizeValue}
          />
          <Dropdown
            title={this.props.defaultRequestSizeUnit}


If we only allow one value for selection, why not disable this dropdown field for now?

@shirimordechay make sense. Will do the changes.

zherman0 · 2019-08-20T18:53:55Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/ocs-install.tsx

+export const CreateOCSServiceForm: React.FC<CreateOCSServiceFormProps> = React.memo((props) => {
+  const title = 'Create New OCS Service';
+  const dropdownUnits = {
+    TiB: 'TiB',


Not much of a dropdown here if there is only 1 option. I know this handles decimals but are you sure you would not like at least GiB?

If there is only one item, should it really be a dropdown?

We are no longer going to show capacity field. Hence, will remove.

zherman0

Looks good. Nice to see hooks being used.

frontend/packages/ceph-storage-plugin/src/components/ocs-install/node-list.tsx

spadgett · 2019-08-20T19:21:00Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/node-list.tsx

+const tableColumnClasses = [
+  classNames('col-md-1', 'col-sm-1', 'col-xs-1'),
+  classNames('col-md-5', 'col-sm-6', 'col-xs-8'),
+  classNames('col-md-2', 'col-sm-3', 'hidden-2'),


hidden-2? Typo?

it should be col-xs-2. Will fix it

spadgett · 2019-08-20T19:21:52Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/node-list.tsx

+  classNames('col-md-5', 'col-sm-6', 'col-xs-8'),
+  classNames('col-md-2', 'col-sm-3', 'hidden-2'),
+  classNames('col-md-2', 'hidden-sm', 'hidden-xs'),
+  classNames('col-md-2', 'hidden-2', 'hidden-xs'),


hidden-2?

it should be col-sm-2. Will fix it

frontend/packages/ceph-storage-plugin/src/components/ocs-install/node-list.tsx

spadgett · 2019-08-20T19:24:16Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/node-list.tsx

+
+const getConvertedUnits = (value, initialUnit, preferredUnit) => {
+  return (
+    humanizeBinaryBytes(_.slice(value, 0, value.length - 2).join(''), initialUnit, preferredUnit)


We should not assume certain units. Instead use the units.js utilities for parsing and displaying memory values.

spadgett · 2019-08-20T20:02:11Z

frontend/packages/ceph-storage-plugin/src/plugin.ts

+    type: 'Page/Route',
+    properties: {
+      exact: true,
+      path: `/k8s/ns/:ns/${ClusterServiceVersionModel.plural}/:appName/${apiObjectRef}/~new`,


It would be better to provide a first-class extension point for creation forms in a future follow-on.

spadgett · 2019-08-20T20:02:53Z

frontend/public/components/utils/storage-class-dropdown.tsx

@@ -8,7 +8,7 @@ import { isDefaultClass } from '../storage-class';

 /* Component StorageClassDropdown - creates a dropdown list of storage classes */

-class StorageClassDropdown_ extends React.Component<StorageClassDropdownProps, StorageClassDropdownState> {
+export class StorageClassDropdown_ extends React.Component<StorageClassDropdownProps, StorageClassDropdownState> {


We should rename this if we are going to export. The trailing underscore implies it's internal.

spadgett · 2019-08-20T20:05:53Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/ocs-install.tsx

+
+  return (
+    <CreateYAML
+      {...props as any}


Why do we need as any here? This usually means a type declaration is wrong somewhere.

spadgett · 2019-08-20T20:09:45Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/ocs-install.tsx

+              breadcrumbs={[
+                {
+                  name: clusterServiceVersion.spec.displayName,
+                  path: window.location.pathname.replace('/~new', ''),


I'd rather not use path math here. Is there a helper for generating the path from the resource?

spadgett · 2019-08-20T20:10:01Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/ocs-install.tsx

+                  name: clusterServiceVersion.spec.displayName,
+                  path: window.location.pathname.replace('/~new', ''),
+                },
+                { name: `Create ${OCSServiceModel.label}`, path: window.location.pathname },


Typically the last breadcrumb is not a link.

the last breadcrumb is not getting displayed as link here.

eparis · 2019-08-22T13:58:36Z

/hold
there is currently no exception request for such a late new feature

julienlim · 2019-08-22T15:54:40Z

@eparis An exception request has just been filed -- pending review and approval we hope!

cloudbehl · 2019-08-22T16:30:16Z

/retest

spadgett · 2019-08-22T16:07:36Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/create-ocs-service.tsx

+              breadcrumbs={[
+                {
+                  name: clusterServiceVersion.spec.displayName,
+                  path: window.location.pathname.replace('/~new', ''),


I don't think this handles custom context roots correctly. You should be using props.match.url from react-router

https://reacttraining.com/react-router/web/api/match

Suggested change

path: window.location.pathname.replace('/~new', ''),

path: props.match.url.replace('/~new', ''),

spadgett · 2019-08-22T16:09:08Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/create-ocs-service.tsx

+                  name: clusterServiceVersion.spec.displayName,
+                  path: window.location.pathname.replace('/~new', ''),
+                },
+                { name: `Create ${OCSServiceModel.label}`, path: window.location.pathname },


Suggested change

{ name: `Create ${OCSServiceModel.label}`, path: window.location.pathname },

{ name: `Create ${OCSServiceModel.label}`, path: props.match.url },

spadgett · 2019-08-22T16:14:56Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/node-list.tsx

+        title: roles.join(', ') || '-',
+      },
+      {
+        title: `${humanizeCpuCores(_.get(node.status, 'capacity.cpu')).string || '-'} CPU`,


The trailing "CPU" is not needed. The column header is "CPU" and humanizeCpuCores handles the units.

+1 that trailing CPU not needed. Should be showing the unit, e.g. MHz, GHz, etc. instead if possible as it's unclear what the unit is oitherwise.

@gnehapk @shirimordechay @spadgett

We can remove the trailing if the header is fixed (fixing the row header as a user scroll provides context on what column the user is on)

spadgett · 2019-08-22T16:15:13Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/node-list.tsx

+        title: `${humanizeCpuCores(_.get(node.status, 'capacity.cpu')).string || '-'} CPU`,
+      },
+      {
+        title: `${getConvertedUnits(_.get(node.status, 'allocatable.memory'), 'KiB')}`,


Use the existing helpers in units.js instead of writing your own.

@spadgett I am using humanizeBinaryBytes utils method inside getConvertedUnits. Are you referring to some other function?

Yes. This function is assuming that a certain unit is used in the YAML. We can't make that assumption. We need to use dehumanize to convert to the base value, and then humanizeBinaryBytes to display.

We could just show the value as-is and humanize in a follow up if needed.

spadgett · 2019-08-22T16:16:03Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/node-list.tsx

+  });
+};
+
+const getAllTableFilters = () => ({


Why is this helper needed? Why not just use tableFilters directly?

Missing types

spadgett · 2019-08-22T16:38:09Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/node-list.tsx

+import { NodeModel } from '@console/internal/models';
+import { Tooltip } from '@console/internal/components/utils/tooltip';
+import { OCSServiceModel } from '../../models';
+import { minSelectedNode, labelObj, ocsRequestData, taintObj } from '../../constants/ocs-install';


labelObj, ocsRequestData, and taintObj are so tightly coupled to this component, I feel like they are better in this file. I sort of disagree with the earlier comment.

@spadgett The intention was to keep all the constants related to ocs install view separately as it was making the file very lengthy. There are so many constants being used here.

spadgett · 2019-08-22T16:39:40Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/node-list.tsx

+  referenceForModel,
+} from '@console/internal/module/k8s';
+import { NodeModel } from '@console/internal/models';
+import { Tooltip } from '@console/internal/components/utils/tooltip';


This is removed in #2455. Use the PF4 tooltip

frontend/packages/ceph-storage-plugin/src/components/ocs-install/node-list.tsx

spadgett · 2019-08-22T16:41:23Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/node-list.tsx

+
+type ocsPropsType = {
+  namespace: string;
+  clusterServiceVersion: { metadata: { name: string } };


Suggested change

clusterServiceVersion: { metadata: { name: string } };

clusterServiceVersion: K8sResourceKind;

spadgett · 2019-08-22T16:42:34Z

frontend/public/components/utils/storage-class-dropdown.tsx

@@ -8,7 +8,7 @@ import { isDefaultClass } from '../storage-class';

 /* Component StorageClassDropdown - creates a dropdown list of storage classes */

-class StorageClassDropdown_ extends React.Component<StorageClassDropdownProps, StorageClassDropdownState> {
+export class SCDropdown extends React.Component<StorageClassDropdownProps, StorageClassDropdownState> {


Exporting both StorageClassDropdown and SCDropdown is confusing. You need to give better names to make the difference clear.

gnehapk · 2019-08-26T13:23:33Z

@spadgett Amended the PR. Please review.

gnehapk · 2019-08-26T14:24:39Z

/retest

spadgett

I've added some additional comments. Thanks for your patience.

The text filter that shows up above the node table was broken when I tried it.

spadgett · 2019-08-26T20:53:39Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/node-list.tsx

+    return objects;
+  }
+
+  const allTableFilters = { ...tableFilters };


Why make a copy of tableFilters? It doesn't look like we're modifying it.

spadgett · 2019-08-26T23:29:39Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/create-form.tsx

+            the OCS Service.
+          </div>
+          <Alert
+            className="co-alert ceph-ocs-info__alert"


Why do we need a custom class? It seems to have broken the alert formatting. We shouldn't be overriding the Patternfly alert styles.

You will need to add the isInline prop

spadgett · 2019-08-26T23:31:15Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/ocs-install.scss

+    }
+  }
+  .ceph-ocs-install__form {
+    max-width: 66%;


Why do we have this style? It's inconsistent with other forms. Please use the standard co-m-pane__form class.

yes, this can be removed.

spadgett · 2019-08-26T23:31:43Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/ocs-install.scss

+    max-width: 66%;
+  }
+
+  .ceph-yaml__link {  position: absolute;  margin-top: 1.5rem;  margin-left: 60%;}


Use the normal co-m-pane__heading-link instead

@spadgett adding co-m-pane__heading-link class is not working correctly as the yaml link is not inside the form. Its outside it and it need to be right aligned. Ataching screenshots after adding co-m-pane__heading-link class and how it should look actually.

It should look like this -

After adding co-m-pane__heading-link class -

You need to move the element to match the expected page structure. Here is an example:

https://github.com/openshift/console/blob/master/frontend/public/components/routes/create-route.tsx#L221-L228

We shouldn't create custom classes for this as we have a general class for all forms with links like this.

spadgett · 2019-08-26T23:32:28Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/ocs-install.scss

@@ -0,0 +1,22 @@
+.ceph-ocs-info__alert {


I believe all of these custom styles can be removed since we have standard styles for everything. We should be able to delete this file.

spadgett · 2019-08-26T23:59:08Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/create-form.tsx

+          <label className="co-required" htmlFor="select-node-help">
+            Select Nodes
+          </label>
+          <div className="control-label help-block" id="select-node-help">


I'd use co-m-pane__explanation here, which will give you padding above the alert. It's not actually a control-label.

Suggested change

<div className="control-label help-block" id="select-node-help">

<p className="co-m-pane__explanation">

spadgett · 2019-08-27T00:04:23Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/node-list.tsx

+      .then((infra) => {
+        // find infra supported provisioner
+        provisioner = infraProvisionerMap[_.lowerCase(_.get(infra, 'status.platform'))];
+        return k8sGet(StorageClassModel);


This should be a k8sList not a k8sGet

spadgett · 2019-08-27T00:06:45Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/node-list.tsx

+    let provisioner = [];
+
+    k8sGet(InfrastructureModel, 'cluster')
+      .then((infra) => {


Suggested change

.then((infra) => {

.then((infra: K8sResourceKinde) => {

spadgett · 2019-08-27T00:07:18Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/node-list.tsx

+        provisioner = infraProvisionerMap[_.lowerCase(_.get(infra, 'status.platform'))];
+        return k8sGet(StorageClassModel);
+      })
+      .then((scData) => {


Assuming you switch to k8sList, this should be

Suggested change

.then((scData) => {

.then((storageClasses: K8sResourceKind[]) => {

spadgett · 2019-08-27T00:09:52Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/storage-class-dropdown.tsx

+const cephStorageProvisioners = ['ceph.rook.io/block', 'cephfs.csi.ceph.com', 'rbd.csi.ceph.com'];
+
+export const OCSStorageClassDropdown: React.FC<OCSStorageClassDropdownProps> = (props) => (
+  <Firehose resources={[{ kind: 'StorageClass', prop: 'StorageClass', isList: true }]}>


nit: prefer storageClass as the prop

gnehapk · 2019-08-27T09:12:32Z

@spadgett Updated the PR. Please review. Also, I have checked the text filter. It seems to be working. Attaching gif for the same.

OCS install flow looks like this -

gnehapk · 2019-08-27T10:56:31Z

/retest

spadgett · 2019-08-27T16:55:16Z

Also, I have checked the text filter. It seems to be working.

Sorry, the filter is working. But it seems to clear the selected nodes.

spadgett · 2019-08-27T16:57:53Z

We really should have a max-height on the table. If you have dozens or hundreds of nodes, the table will keep growing and push the buttons far down on the page.

spadgett · 2019-08-27T17:00:54Z

It's a little odd that both "Select Nodes" and "Select at least 3 nodes you wish to use" have the required asterick, but will defer to @julienlim

spadgett · 2019-08-27T17:22:49Z

Table layout is broken at mobile:

spadgett · 2019-08-27T17:23:19Z

I'd suggest not using the grid breakpoint and just dropping to two columns at mobile

gnehapk · 2019-08-28T06:51:57Z

It's a little odd that both "Select Nodes" and "Select at least 3 nodes you wish to use" have the required asterick, but will defer to @julienlim

As per the latest design, the asterick has been removed from "Select Nodes" heading. Will remove it.

gnehapk · 2019-08-28T15:54:32Z

Also, I have checked the text filter. It seems to be working.

Sorry, the filter is working. But it seems to clear the selected nodes.

We are working on this issue. Planning to send a separate PR for this bug fix.

gnehapk · 2019-08-28T15:58:58Z

Table layout is broken at mobile:

I have modified the code to show 2 columns now. It looks like this now -

I verified the table layout of patternfly react table as well on mobile view, and it looks like this only. Attaching its screenshots as well.

cloudbehl · 2019-08-28T17:48:44Z

/test e2e-aws-console

spadgett · 2019-08-28T20:31:40Z

For the table, I was suggesting gridBreakPoint={TableGridBreakpoint.none} to avoid the special mobile view, which we don't use in console currently.

spadgett · 2019-08-28T23:39:12Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/ocs-install.scss

+}
+
+.node-list__max-height {
+  max-height: 50vh;


We shouldn't use vh... This will be tiny on mobile. Just pick an absolute value in px

spadgett · 2019-08-28T23:41:07Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/ocs-install.scss

+.ceph-yaml__link {
+  position: absolute;
+  margin-top: 1.5rem;
+  margin-left: 50%;


This is going to be in the wrong spot at mobile... We can clean up in a follow on, however.

Noted. Will address in follow up PR.

spadgett · 2019-08-28T23:45:58Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/node-list.tsx

+    const labelPath = '/metadata/labels';
+    const labelNodesRequest = selectedNode.map((node: NodeKind) => {
+      const labels = SelectorInput.arrayify(_.get(node, labelPath.split('/').slice(1)));
+      const lblVal = { ...SelectorInput.objectify(labels), ...labelObj };


You shouldn't be using SelectorInput if you're not using the label selector widget....

Just patch the one label value you want.

{op: 'add', path: 'metadata/labels/cluster.ocs.openshift.io~1openshift-storage', value: ''}

This is all you need... You should be able to remove all the other logic, and it will avoid possibly overwriting another label change if it comes in at almost the same time. The whole function can be:

const labelNodes = (selectedNodes: NodeKind[]): Promise<NodeKind>[] => { return selectedNodes.map((node: NodeKind) => { const patch = {op: 'add', path: 'metadata/labels/cluster.ocs.openshift.io~1openshift-storage', value: ''}; return k8sPatch(NodeModel, node, patch); }); };

spadgett · 2019-08-28T23:59:16Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/storage-class-dropdown.tsx

+  </Firehose>
+);
+
+const StorageClassDropdown = (props) => {


This would be much more straightforward to implement if you had simply passed an optional filter prop the existing dropdown. Then you wouldn't have had to create your own wrapper, and it's a small code change.

We can address in a tech debt follow on.

sure, will address in follow on. Thanks!

gnehapk · 2019-08-29T13:39:44Z

@spadgett I have amended the PR. Please review.

spadgett

/approve
/lgtm

Let's work on follow up PRs for the outstanding code review feedback. Minimally, we should fix the bug where filtering the list clears the selection.

spadgett · 2019-08-29T14:41:41Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/node-list.tsx

+      selected: false,
+      id: node.metadata.name,
+      metadata: _.clone(node.metadata),
+      spec: _.clone(node.spec),


Do we need _.clone? Simply

...node,

is probably better unless we are mutating the values.

spadgett · 2019-08-29T14:44:27Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/node-list.tsx

+      setNodes(formattedNodes);
+    }
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [JSON.stringify(data), loaded]);


I don't think you need JSON.stringify here. If data is not the same reference, it means it was updated through Firehose and the data is changed. Are you seeing something different?

Suggested change

}, [JSON.stringify(data), loaded]);

}, [data, loaded]);

spadgett · 2019-08-29T14:47:41Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/node-list.tsx

+import './ocs-install.scss';
+
+const ocsLabel = 'cluster.ocs.openshift.io/openshift-storage';
+const nodeLabel = 'cluster.ocs.openshift.io~1openshift-storage';


Just inline this below. This is only used in one place and should only be used for the specific patch request below since it has the JSON path escaping. It's not obvious when looking at the patch request that things are correctly escaped.

spadgett · 2019-08-29T14:48:42Z

frontend/packages/ceph-storage-plugin/src/components/ocs-install/node-list.tsx

+
+const ocsLabel = 'cluster.ocs.openshift.io/openshift-storage';
+const nodeLabel = 'cluster.ocs.openshift.io~1openshift-storage';
+const defaultSAnotations = { 'storageclass.kubernetes.io/is-default-class': 'true' };


Ideally we'd have one constant for this.

console/frontend/public/components/storage-class.tsx

Line 14 in e5bcd04

const defaultClassAnnotation = 'storageclass.kubernetes.io/is-default-class';

We can address in a follow on.

openshift-ci-robot · 2019-08-29T14:53:47Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gnehapk, spadgett, zherman0

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~~ [spadgett]

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

spadgett · 2019-08-29T15:14:27Z

/hold cancel

cloudbehl · 2019-08-29T16:32:15Z

/test e2e-aws-console-olm

openshift-bot · 2019-08-29T18:59:40Z

/retest

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

openshift-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Aug 15, 2019

openshift-ci-robot requested review from jhadvig and mareklibra August 15, 2019 06:02

openshift-ci-robot added component/ceph Related to ceph-storage-plugin component/core Related to console core functionality labels Aug 15, 2019

gnehapk mentioned this pull request Aug 15, 2019

[WIP] OCS Service installtion flow #1982

Closed

priley86 reviewed Aug 15, 2019

View reviewed changes

umangachapagain reviewed Aug 16, 2019

View reviewed changes

shirimordechay reviewed Aug 18, 2019

View reviewed changes

gnehapk force-pushed the ocs-install-v2 branch from bcf7399 to f6236d4 Compare August 19, 2019 13:11

julienlim mentioned this pull request Aug 20, 2019

OCS Installation UI flow openshift/openshift-origin-design#211

Merged

zherman0 reviewed Aug 20, 2019

View reviewed changes

zherman0 approved these changes Aug 20, 2019

View reviewed changes

spadgett requested changes Aug 20, 2019

View reviewed changes

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 22, 2019

gnehapk force-pushed the ocs-install-v2 branch from f6236d4 to 42869f7 Compare August 22, 2019 14:58

spadgett requested changes Aug 22, 2019

View reviewed changes

gnehapk force-pushed the ocs-install-v2 branch from 42869f7 to b705e52 Compare August 26, 2019 13:21

a2batic mentioned this pull request Aug 26, 2019

Feature: Expand cluster modal #2348

Merged

spadgett requested changes Aug 27, 2019

View reviewed changes

gnehapk force-pushed the ocs-install-v2 branch from b705e52 to 4bbc156 Compare August 27, 2019 09:09

gnehapk force-pushed the ocs-install-v2 branch from 4bbc156 to f7764df Compare August 28, 2019 15:52

spadgett reviewed Aug 29, 2019

View reviewed changes

gnehapk force-pushed the ocs-install-v2 branch 2 times, most recently from 3b6ef01 to 42bf875 Compare August 29, 2019 13:38

Create OCS Service workflow

4d95476

gnehapk force-pushed the ocs-install-v2 branch from 42bf875 to 4d95476 Compare August 29, 2019 14:06

spadgett approved these changes Aug 29, 2019

View reviewed changes

openshift-ci-robot assigned spadgett Aug 29, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 29, 2019

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 29, 2019

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 29, 2019

openshift-merge-robot merged commit 066095e into openshift:master Aug 29, 2019

spadgett added this to the v4.2 milestone Aug 30, 2019

atiratree mentioned this pull request Sep 12, 2019

table: do not pass customData as rows #2701

Merged

gnehapk mentioned this pull request Sep 27, 2019

Bug 1766587: Follow up changes from ocs install PR #2854

Merged

		@@ -230,13 +230,13 @@ const VirtualBody: React.SFC<VirtualBodyProps> = (props) => {
		);

	path: window.location.pathname.replace('/~new', ''),
	path: props.match.url.replace('/~new', ''),

	{ name: `Create ${OCSServiceModel.label}`, path: window.location.pathname },
	{ name: `Create ${OCSServiceModel.label}`, path: props.match.url },

	clusterServiceVersion: { metadata: { name: string } };
	clusterServiceVersion: K8sResourceKind;

	<div className="control-label help-block" id="select-node-help">
	<p className="co-m-pane__explanation">

	.then((scData) => {
	.then((storageClasses: K8sResourceKind[]) => {

Create OCS Service workflow #2359

Create OCS Service workflow #2359

Conversation

gnehapk commented Aug 15, 2019

priley86 Aug 15, 2019 • edited Loading

Choose a reason for hiding this comment

lizsurette commented Aug 15, 2019

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

gnehapk Aug 19, 2019 • edited Loading

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

zherman0 left a 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

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

eparis commented Aug 22, 2019

julienlim commented Aug 22, 2019

cloudbehl commented Aug 22, 2019

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spadgett Aug 26, 2019 • edited Loading

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

Choose a reason for hiding this comment

gnehapk commented Aug 26, 2019

gnehapk commented Aug 26, 2019

spadgett left a 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

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

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

gnehapk commented Aug 27, 2019

priley86 Aug 15, 2019 •

edited

Loading

gnehapk Aug 19, 2019 •

edited

Loading

spadgett Aug 26, 2019 •

edited

Loading