Note: This bug is displayed in read-only format because the product is no longer active in Red Hat Bugzilla.

Bug 1867824

Summary:

The "ordering of fields" on Operand creation form does not behave as intended

Product:

OpenShift Container Platform

Reporter:

tony.wu

Component:

Management Console

Assignee:

Jon Jackson <jonjacks>

Status:

CLOSED ERRATA

QA Contact:

Yadan Pei <yapei>

Severity:

medium

Docs Contact:

Priority:

medium

Version:

4.6

CC:

aos-bugs, jokerman, jonjacks, yapei

Target Milestone:

---

Target Release:

4.6.0

Hardware:

Unspecified

OS:

Unspecified

Whiteboard:

Fixed In Version:

Doc Type:

If docs needed, set a value

Doc Text:

Story Points:

---

Clone Of:

Environment:

Last Closed:

2020-10-27 16:27:30 UTC

Type:

Bug

Regression:

---

Mount Type:

---

Documentation:

---

CRM:

Verified Versions:

Category:

---

oVirt Team:

---

RHEL 7.3 requirements from Atomic Host:

Cloudforms Team:

---

Target Upstream Version:

Embargoed:

Attachments:

Description	Flags
0_specDescriptors_ordering__portworx_CSV_for_storageCluster@2x.png	none
1_current__portworx_storageCluster_form@2x.png	none
2_in_specDescriptors_ordering__portworx_storageCluster_form@2x.png	none
Storage Cluster Creation Form	none
Storage Cluster Creation Form Correct Order	none
Correct Order in Form when all field group have correct path	none

Description tony.wu 2020-08-10 22:16:45 UTC

Created attachment 1711013 [details]
0_specDescriptors_ordering__portworx_CSV_for_storageCluster

Description of problem:

Migrated from Jira bug:
https://issues.redhat.com/browse/CONSOLE-1815

When checking the Operator's specDescriptors in the CSV file, it's noticed the form field ordering is not as it intended to be. 

See "Ordering of Form Fields" details in the doc:
https://github.com/openshift/console/tree/master/frontend/packages/operator-lifecycle-manager/src/components/descriptors#ordering-of-form-fields
---
The ordering of form fields depends on whether a field has a specDescriptor, is "required" (a required: property in the schema), "optional", or "advanced" (assigned with an advanced specDecriptor). In general, fields with specDescriptors will be sorted higher than those without and required fields will be sorted higher than optional fields. The full set of sorting rules are as follows:
1. "required" fields with specDescriptors other than advanced
2. "required" fields without specDescriptors
3. "optional" fields with specDescriptors other than advanced
4. "optional" fields without specDescriptors
5. Fields with advanced specDescriptors wrapped in the 'Advanced Configuration' group
---


Version-Release number of selected component (if applicable):
OCP 4.6 and 4.5

How reproducible:
100%

Steps to Reproduce:
1. Install "Portworx Operator" from 'OperatorHub' page (as an example)
2. Go to 'Installed Operators' page and click on '+Create instance' link for "StorageCluster" Operand
3. See the creation form view

Actual results:
See the attached image: 1_current__portworx_storageCluster_form


Expected results:
See the attached image: 2_in_specDescriptors_ordering__portworx_storageCluster_form

as the specDescriptors in Portworx Operator's CSV is as in:
0_specDescriptors_ordering__portworx_CSV_for_storageCluster


Additional info:

Comment 1 tony.wu 2020-08-10 22:17:05 UTC

Created attachment 1711014 [details]
1_current__portworx_storageCluster_form

Comment 2 tony.wu 2020-08-10 22:17:16 UTC

Created attachment 1711015 [details]
2_in_specDescriptors_ordering__portworx_storageCluster_form

Comment 5 Yadan Pei 2020-08-26 08:39:57 UTC

Hi Tony,

Could you please tell me how to get 'Portworx Operator' ? By default the operator is not showing up in OperatorHub in 4.6. I think Portworx Operator maybe a perfect operator to verify this bug.

Comment 6 Yadan Pei 2020-08-31 07:24:08 UTC

Given following specDescriptors for StorageCluster: 
        specDescriptors:
          - description: The docker image name and version of Portworx Enterprise.
            displayName: Image   ==> 1
            path: image
            x-descriptors:
              - 'urn:alm:descriptor:com.tectonic.ui:text'
          - description: >-
              It is a reference to a secret in the same namespace as the
              StorageCluster. This secret is used to pull images from a private
              registry.
            displayName: Private Registry Image Pull Secret. ==> 2 
            path: imagePullSecret
            x-descriptors:
              - 'urn:alm:descriptor:io.kubernetes:Secret'
              - 'urn:alm:descriptor:com.tectonic.ui:fieldGroup:image'
          - description: >-
              CustomImageRegistry is a custom container registry server (may
              include repository) that will be used instead of index.docker.io
              to download Docker images. (Example: myregistry.net:5443 or
              myregistry.com/myrepository)
            displayName: Custom Image Registry ==> 3
            path: customImageRegistry
            x-descriptors:
              - 'urn:alm:descriptor:com.tectonic.ui:text'
              - 'urn:alm:descriptor:com.tectonic.ui:fieldGroup:image'
          - description: >-
              Autopilot automatically expands PVCs and Portworx storage based on
              the current usage.
            displayName: Enable Autopilot  ==> grouping in autopilot
            path: autopilot.enable
            x-descriptors:
              - 'urn:alm:descriptor:com.tectonic.ui:advanced'
              - 'urn:alm:descriptor:com.tectonic.ui:booleanSwitch'
          - description: >-
              The secrets provider which will contain secrets that are needed by
              Portworx for features like volume encryption, cloudsnaps, etc.
            displayName: Secrets Provider 
            path: secretsProvider
            x-descriptors:
              - 'urn:alm:descriptor:com.tectonic.ui:advanced'
              - 'urn:alm:descriptor:com.tectonic.ui:select:k8s'
              - 'urn:alm:descriptor:com.tectonic.ui:select:vault'
              - 'urn:alm:descriptor:com.tectonic.ui:select:aws-kms'
              - 'urn:alm:descriptor:com.tectonic.ui:select:azure-kv'
              - 'urn:alm:descriptor:com.tectonic.ui:select:ibm-kp'
          - description: This controls the delete strategy of the Portworx cluster.
            displayName: DeleteStrategy  ==> grouping in deletestrategy
            path: deleteStrategy.type
            x-descriptors:
              - 'urn:alm:descriptor:com.tectonic.ui:advanced'
              - 'urn:alm:descriptor:com.tectonic.ui:select:Uninstall'
              - 'urn:alm:descriptor:com.tectonic.ui:select:UninstallAndWipe'
          - description: >-
              It is the pull policy for the image. Accepts one of Always, Never,
              IfNotPresent. Defaults to Always.
            displayName: Image Pull Policy   ==> 4
            path: imagePullPolicy
          - description: Expose Portworx metrics to Prometheus.
            displayName: Enable Metrics  ==> grouping in Monitoring
            path: monitoring.enableMetrics
          - description: The network interface to be used by Portworx for data traffic.
            displayName: Data Interface  ==> grouping in Network
            path: network.dataInterface
          - description: >-
              The network interface to be used by Portworx for management
              traffic.
            displayName: Management Interface ==> grouping in Network
            path: network.mgmtInterface
          - description: It is the starting port in the range of ports used by Portworx.
            displayName: Start Port  ==> 5
            path: startPort
          - description: Revision history limit is the number of old histories to retain.
            displayName: Revision History Limit ==> 6
            path: revisionHistoryLimit
          - description: >-
              Version is a read-only field. It contains the current version of
              Portworx.
            displayName: Version ==> 7
            path: version


There are three fields with 'urn:alm:descriptor:com.tectonic.ui:advanced' descriptors and the order is:
          - description: >-
              Autopilot automatically expands PVCs and Portworx storage based on
              the current usage.
            displayName: Enable Autopilot  ==> grouping in autopilot
            path: autopilot.enable
            x-descriptors:
              - 'urn:alm:descriptor:com.tectonic.ui:advanced'
              - 'urn:alm:descriptor:com.tectonic.ui:booleanSwitch'
          - description: >-
              The secrets provider which will contain secrets that are needed by
              Portworx for features like volume encryption, cloudsnaps, etc.
            displayName: Secrets Provider 
            path: secretsProvider
            x-descriptors:
              - 'urn:alm:descriptor:com.tectonic.ui:advanced'
              - 'urn:alm:descriptor:com.tectonic.ui:select:k8s'
              - 'urn:alm:descriptor:com.tectonic.ui:select:vault'
              - 'urn:alm:descriptor:com.tectonic.ui:select:aws-kms'
              - 'urn:alm:descriptor:com.tectonic.ui:select:azure-kv'
              - 'urn:alm:descriptor:com.tectonic.ui:select:ibm-kp'
          - description: This controls the delete strategy of the Portworx cluster.
            displayName: DeleteStrategy  ==> grouping in deletestrategy
            path: deleteStrategy.type
            x-descriptors:
              - 'urn:alm:descriptor:com.tectonic.ui:advanced'
              - 'urn:alm:descriptor:com.tectonic.ui:select:Uninstall'
              - 'urn:alm:descriptor:com.tectonic.ui:select:UninstallAndWipe'

in the operand creation form page, the correct order based on my understanding should be : Autopilot -> Delete Strategy -> Secrets Provider comes at the bottom of the form in 'Advanced Options', but the curret order is: Delete Strategy -> Autopilot -> Secrets Provider comes at the bottom of the form in 'Advanced Options'

@Jon, can you help confirm?

Comment 7 Yadan Pei 2020-08-31 07:24:54 UTC

Created attachment 1713117 [details]
Storage Cluster Creation Form

Comment 8 Jon Jackson 2020-09-01 13:15:05 UTC

(In reply to Yadan Pei from comment #6)
> 
> in the operand creation form page, the correct order based on my
> understanding should be : Autopilot -> Delete Strategy -> Secrets Provider
> comes at the bottom of the form in 'Advanced Options', but the curret order
> is: Delete Strategy -> Autopilot -> Secrets Provider comes at the bottom of
> the form in 'Advanced Options'
> 
> @Jon, can you help confirm?

Advanced descriptors are a special case. The normal sorting rules do not apply. The advanced descriptor will cause the target field to be rendered in an expandable "Advanced Configuration" section which is always rendered at the bottom of it's direct parent.

Comment 9 tony.wu 2020-09-01 20:24:10 UTC

Hey Yadan, 

As Jon mentioned, the fields with "Advanced" descriptor will be rendered at the bottom of its direct parent field group.

You can see more details in
https://github.com/openshift/console/tree/master/frontend/packages/operator-lifecycle-manager/src/components/descriptors#ordering-of-form-fields


Hence, in the examples (with advanced) you attached:

1. 'autopilot.enable' with advanced specDescriptor
--> The 'enable' booleanSwitch field should be rendered in an expandable "Advanced Configuration" section within "Autopilot" Accordion. 
--> @Jon, this seems to be a bug here as the "Advanced Configuration" section isn't showing up within "Autopilot" Accordion. 

2. 'secretsProvider' with advanced specDescriptor
--> The 'secretsProvider' dropdown select menu is rendered in an expandable "Advanced Configuration" section at the "top-level" object, which is the Operand instance itself, i.e. essentially the bottom of this creation form.

3. 'deleteStrategy.type' with advanced specDescriptor
--> The 'deleteStrategy.type' (displayName is a bit confusing as the "DeleteStrategy") dropdown select menu is rendered in an expandable "Advanced Configuration" section at the bottom of its direct parent "Delete Strategy" group.

Comment 10 Yadan Pei 2020-09-02 07:35:15 UTC

Thanks very much Jon and Tony.

I now understand `the fields with "Advanced" descriptor will be rendered at the bottom of its direct parent field group.`. 

I have another question that for the parent field group 'Delete Strategy' and 'Authpolit', why are they in a order of Delete Strategy -> Autopilot, not Autopilot -> Delete Strategy? Since in both CRD schema and CSV specDescriptors, Autopilot seems comes first then Delete Strategy.

Comment 11 Jon Jackson 2020-09-03 17:11:18 UTC

Tony and Yadan, I looked into the issues you both pointed out (field ordering incorrect, Autopilot missing 'Advanced Configuration') and it looks like the root cause for both is a bug in the CSV itself. The descriptor intended for the Autopilot -> Enabled field has an incorrect path. The correct schema path to that field is 'autopilot.enabled', but the CSV defines it as 'autopilot.enable'. Updating the path fixes both issues.

Comment 12 tony.wu 2020-09-03 17:47:19 UTC

It seems that the CSV does use 'autopilot.enabled' as the path. Jon can you help clarify this again in terms of the ordering of the fields? Much appreciated!

Comment 13 Yadan Pei 2020-09-07 02:27:04 UTC

Created attachment 1713909 [details]
Storage Cluster Creation Form Correct Order

After updating patch from 'autopilot.enable' to 'autopilot.enabled', the order becomes Autopilot -> Delete Strategy, and Advanced Configuration in Autopilot is rendered.

Comment 14 Yadan Pei 2020-09-08 01:49:16 UTC

It looks like the issue reported has been fixed. moving to VERIFIED

Let me know if it is wrong

Comment 15 tony.wu 2020-09-08 19:23:08 UTC

Hi Yadan, 

I've chatted with Jon on Sep 3rd and clarified the goal for the "ordering of fields" on Operand creation form is "a parent is considered to have a descriptor if any of it’s children do". You can see the "Example case" in the original JIRA bug for reference:
https://issues.redhat.com/browse/CONSOLE-1815 

Hence, the "Create StorageCluster" form should look like as in the attached mocked screenshot: 2_in_specDescriptors_ordering__portworx_storageCluster_form

Jon will be working on a PR later for this so he'll need to tag this BZ. Can you help verify that after the new PR? thanks

Comment 16 Yadan Pei 2020-09-09 01:31:31 UTC

Sure,

Let me move it back to ASSIGNED to catch my attention

Comment 17 Jon Jackson 2020-09-11 21:10:43 UTC

PR is in merge queue.

Comment 19 Yadan Pei 2020-09-15 07:11:56 UTC

1. When specDescriptors is in following order: 
        specDescriptors:
          - description: The docker image name and version of Portworx Enterprise.
            displayName: Image
            path: image
            x-descriptors:
              - 'urn:alm:descriptor:com.tectonic.ui:text'
          - description: >-
              It is a reference to a secret in the same namespace as the
              StorageCluster. This secret is used to pull images from a private
              registry.
            displayName: Private Registry Image Pull Secret
            path: imagePullSecret
            x-descriptors:
              - 'urn:alm:descriptor:io.kubernetes:Secret'
              - 'urn:alm:descriptor:com.tectonic.ui:fieldGroup:image'
          - description: >-
              CustomImageRegistry is a custom container registry server (may
              include repository) that will be used instead of index.docker.io
              to download Docker images. (Example: myregistry.net:5443 or
              myregistry.com/myrepository)
            displayName: Custom Image Registry
            path: customImageRegistry
            x-descriptors:
              - 'urn:alm:descriptor:com.tectonic.ui:text'
              - 'urn:alm:descriptor:com.tectonic.ui:fieldGroup:image'
          - description: >-      
              Autopilot automatically expands PVCs and Portworx storage based on
              the current usage.
            displayName: Enable Autopilot
            path: autopilot.enable         ====> Autopilot group with incorrect path
            x-descriptors:
              - 'urn:alm:descriptor:com.tectonic.ui:advanced'
              - 'urn:alm:descriptor:com.tectonic.ui:booleanSwitch'
          - description: >-
              The secrets provider which will contain secrets that are needed by
              Portworx for features like volume encryption, cloudsnaps, etc.
            displayName: Secrets Provider
            path: secretsProvider
            x-descriptors:
              - 'urn:alm:descriptor:com.tectonic.ui:advanced'
              - 'urn:alm:descriptor:com.tectonic.ui:select:k8s'
              - 'urn:alm:descriptor:com.tectonic.ui:select:vault'
              - 'urn:alm:descriptor:com.tectonic.ui:select:aws-kms'
              - 'urn:alm:descriptor:com.tectonic.ui:select:azure-kv'
              - 'urn:alm:descriptor:com.tectonic.ui:select:ibm-kp'
          - description: This controls the delete strategy of the Portworx cluster.
            displayName: DeleteStrategy  
            path: deleteStrategy.type          ====> DeleteStrategy group with correct path
            x-descriptors:
              - 'urn:alm:descriptor:com.tectonic.ui:advanced'
              - 'urn:alm:descriptor:com.tectonic.ui:select:Uninstall'
              - 'urn:alm:descriptor:com.tectonic.ui:select:UninstallAndWipe'
          - description: >-
              It is the pull policy for the image. Accepts one of Always, Never,
              IfNotPresent. Defaults to Always.
            displayName: Image Pull Policy
            path: imagePullPolicy
          - description: Expose Portworx metrics to Prometheus.   
            displayName: Enable Metrics
            path: monitoring.enableMetrics         ====> Monitoring group with correct path
          - description: The network interface to be used by Portworx for data traffic.   
            displayName: Data Interface
            path: network.dataInterface            ====> Network group with correct path
          - description: >-
              The network interface to be used by Portworx for management
              traffic.
            displayName: Management Interface
            path: network.mgmtInterface        ====> Network group with correct path
          - description: It is the starting port in the range of ports used by Portworx.
            displayName: Start Port
            path: startPort
          - description: Revision history limit is the number of old histories to retain.
            displayName: Revision History Limit
            path: revisionHistoryLimit
          - description: >-
              Version is a read-only field. It contains the current version of
              Portworx.
            displayName: Version
            path: version

The field group order in form view is: DeleteStrategy -> Monitoring -> Network -> Autopilot

2. When specDescritptors is in following order:
        specDescriptors:
          - description: The docker image name and version of Portworx Enterprise.
            displayName: Image
            path: image
            x-descriptors:
              - 'urn:alm:descriptor:com.tectonic.ui:text'
          - description: >-
              It is a reference to a secret in the same namespace as the
              StorageCluster. This secret is used to pull images from a private
              registry.
            displayName: Private Registry Image Pull Secret
            path: imagePullSecret
            x-descriptors:
              - 'urn:alm:descriptor:io.kubernetes:Secret'
              - 'urn:alm:descriptor:com.tectonic.ui:fieldGroup:image'
          - description: >-
              CustomImageRegistry is a custom container registry server (may
              include repository) that will be used instead of index.docker.io
              to download Docker images. (Example: myregistry.net:5443 or
              myregistry.com/myrepository)
            displayName: Custom Image Registry
            path: customImageRegistry
            x-descriptors:
              - 'urn:alm:descriptor:com.tectonic.ui:text'
              - 'urn:alm:descriptor:com.tectonic.ui:fieldGroup:image'
          - description: >-      
              Autopilot automatically expands PVCs and Portworx storage based on
              the current usage.
            displayName: Enable Autopilot
            path: autopilot.enabled         ====> Autopilot group with correct path
            x-descriptors:
              - 'urn:alm:descriptor:com.tectonic.ui:advanced'
              - 'urn:alm:descriptor:com.tectonic.ui:booleanSwitch'
          - description: >-
              The secrets provider which will contain secrets that are needed by
              Portworx for features like volume encryption, cloudsnaps, etc.
            displayName: Secrets Provider
            path: secretsProvider
            x-descriptors:
              - 'urn:alm:descriptor:com.tectonic.ui:advanced'
              - 'urn:alm:descriptor:com.tectonic.ui:select:k8s'
              - 'urn:alm:descriptor:com.tectonic.ui:select:vault'
              - 'urn:alm:descriptor:com.tectonic.ui:select:aws-kms'
              - 'urn:alm:descriptor:com.tectonic.ui:select:azure-kv'
              - 'urn:alm:descriptor:com.tectonic.ui:select:ibm-kp'
          - description: This controls the delete strategy of the Portworx cluster.
            displayName: DeleteStrategy  
            path: deleteStrategy.type          ====> DeleteStrategy group with correct path
            x-descriptors:
              - 'urn:alm:descriptor:com.tectonic.ui:advanced'
              - 'urn:alm:descriptor:com.tectonic.ui:select:Uninstall'
              - 'urn:alm:descriptor:com.tectonic.ui:select:UninstallAndWipe'
          - description: >-
              It is the pull policy for the image. Accepts one of Always, Never,
              IfNotPresent. Defaults to Always.
            displayName: Image Pull Policy
            path: imagePullPolicy
          - description: Expose Portworx metrics to Prometheus.   
            displayName: Enable Metrics
            path: monitoring.enableMetrics         ====> Monitoring group with correct path
          - description: The network interface to be used by Portworx for data traffic.   
            displayName: Data Interface
            path: network.dataInterface            ====> Network group with correct path
          - description: >-
              The network interface to be used by Portworx for management
              traffic.
            displayName: Management Interface
            path: network.mgmtInterface        ====> Network group with correct path
          - description: It is the starting port in the range of ports used by Portworx.
            displayName: Start Port
            path: startPort
          - description: Revision history limit is the number of old histories to retain.
            displayName: Revision History Limit
            path: revisionHistoryLimit
          - description: >-
              Version is a read-only field. It contains the current version of
              Portworx.
            displayName: Version
            path: version
The field group order in form view is: Autopilot -> DeleteStrategy -> Monitoring -> Network 

I think this time we have correct order on form view

This is checked against 4.6.0-0.nightly-2020-09-14-181123. 

Moving to VERIFIED

Comment 20 Yadan Pei 2020-09-15 07:13:16 UTC

Created attachment 1714891 [details]
Correct Order in Form when all field group have correct path

Comment 21 Yadan Pei 2020-09-16 01:34:25 UTC

I forgot to mention that the field group are in correct order( Field groups are NOT all sorted after the rest of the descriptor-defined fields),  they are ordered based on child field descriptor order in descriptor array

Comment 23 errata-xmlrpc 2020-10-27 16:27:30 UTC

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (OpenShift Container Platform 4.6 GA Images), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2020:4196