-
Notifications
You must be signed in to change notification settings - Fork 667
Bug 1766587: Follow up changes from ocs install PR #2854
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 1766587: Follow up changes from ocs install PR #2854
Conversation
| if (sc.metadata && _.isEqual(sc.metadata.annotations, defaultSAnotations)) { | ||
| if ( | ||
| sc.metadata && | ||
| _.isEqual(sc.metadata.annotations, { [defaultClassAnnotation]: 'true' }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should only check the value of the single annotation. Otherwise another annotation on the storage class would cause the check to fail. But better to use the existing isDefaultClass helper from storage.class.tsx.
| @@ -12,7 +12,7 @@ export const StorageClassReference: K8sResourceKindReference = 'StorageClass'; | |||
| const { common } = Kebab.factory; | |||
| const menuActions = [...Kebab.getExtensionsActionsForKind(StorageClassModel), ...common]; | |||
|
|
|||
| const defaultClassAnnotation = 'storageclass.kubernetes.io/is-default-class'; | |||
| export const defaultClassAnnotation = 'storageclass.kubernetes.io/is-default-class'; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't export this, use isDefaultClass below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack
f11bac4 to
1ace0c0
Compare
|
@spadgett Amended the PR. Please review |
frontend/packages/ceph-storage-plugin/src/components/ocs-install/node-list.tsx
Outdated
Show resolved
Hide resolved
| @@ -178,7 +177,7 @@ const CustomNodeTable: React.FC<CustomNodeTableProps> = ({ data, loaded, ocsProp | |||
| setNodes(formattedNodes); | |||
| } | |||
| // eslint-disable-next-line react-hooks/exhaustive-deps | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also needs to be removed if you are removing JSON.stringify.
frontend/packages/ceph-storage-plugin/src/components/ocs-install/node-list.tsx
Outdated
Show resolved
Hide resolved
1ace0c0 to
2e12ac6
Compare
2e12ac6 to
25a7488
Compare
|
/test e2e-aws-console-olm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cloudbehl, gnehapk The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test e2e-aws-console-olm |
|
/cherrypick release-4.2 |
|
@gnehapk: new pull request created: #3124 DetailsIn response to this:
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. |
|
@gnehapk: Bugzilla bug 1766587 is in an unrecognized state (ON_QA) and will not be moved to the MODIFIED state. DetailsIn response to this:
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. |
|
/bugzilla refresh |
|
@gnehapk: Bugzilla bug 1766587 is in an unrecognized state (MODIFIED) and will not be moved to the MODIFIED state. DetailsIn response to this:
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. |
Follow up changes from ocs install PR
@spadgett Please review.