Add supports for non DF storage backends #2155
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: vbnrh The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
There is some issue with ACM search query and couple of issues with CSS due to patternfly version mismatch. Trying to fix that ASAP |
vbnrh
changed the title
vbnrh
force-pushed
the
dfbugs-6793
branch
4 times, most recently
from
d2a98b8 to
5ae745e
Compare
Signed-off-by: vbadrina <[email protected]>
|
@vbnrh: The following test failed, say
Full PR test history. Your PR dashboard. 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-sigs/prow repository. I understand the commands that are listed here. |
| updated.metadata = { | ||
| ...(current as T).metadata, | ||
| ...(updated.metadata || {}), | ||
| resourceVersion: (current as T).metadata?.resourceVersion, |
There was a problem hiding this comment.
why do we need to pass resourceVersion ?? Is it required for k8sUpdate ??
| @@ -12640,7 +12640,7 @@ string-length@^4.0.1, string-length@^4.0.2: | |||
| char-regex "^1.0.2" | |||
| strip-ansi "^6.0.0" | |||
|
|
|||
| "string-width-cjs@npm:string-width@^4.2.0": | |||
There was a problem hiding this comment.
yarn.lock file shouldn't get updated without changes to package.json, can u plz revert this back to what's on latest master ??
There was a problem hiding this comment.
I tested locally on latest master, I don't see this file getting automatically updated...
| @@ -1397,6 +1398,7 @@ | |||
| "Protection name:": "Protection name:", | |||
| "Protection type": "Protection type", | |||
| "Protection type:": "Protection type:", | |||
| "Provide S3 bucket connection details for each managed cluster. If an s3 bucket is not already configured for cluster, create one and then continue.": "Provide S3 bucket connection details for each managed cluster. If an s3 bucket is not already configured for cluster, create one and then continue.", | |||
There was a problem hiding this comment.
| "Provide S3 bucket connection details for each managed cluster. If an s3 bucket is not already configured for cluster, create one and then continue.": "Provide S3 bucket connection details for each managed cluster. If an s3 bucket is not already configured for cluster, create one and then continue.", | |
| "Provide S3 bucket connection details for each managed cluster. If a S3 bucket is not already configured for cluster, create one and then continue.": "Provide S3 bucket connection details for each managed cluster. If a S3 bucket is not already configured for cluster, create one and then continue.", |
| export function fnv1a32(str: string): string { | ||
| /* eslint-disable no-bitwise */ | ||
| let h = 0x811c9dc5; | ||
| for (let i = 0; i < str.length; i++) { | ||
| h ^= str.charCodeAt(i); | ||
| h = (h * 0x01000193) >>> 0; | ||
| } | ||
| /* eslint-enable no-bitwise */ | ||
| return h.toString(16).padStart(8, '0'); | ||
| } |
There was a problem hiding this comment.
use murmurhash-js, we are already using it at other places...
u can also use getRandomChars utils from common...
| export function fnv1a32(str: string): string { | |
| /* eslint-disable no-bitwise */ | |
| let h = 0x811c9dc5; | |
| for (let i = 0; i < str.length; i++) { | |
| h ^= str.charCodeAt(i); | |
| h = (h * 0x01000193) >>> 0; | |
| } | |
| /* eslint-enable no-bitwise */ | |
| return h.toString(16).padStart(8, '0'); | |
| } |
| const areS3ProfileFieldsEqual = ( | ||
| a: S3StoreProfile, | ||
| b: S3StoreProfile | ||
| ): boolean => { | ||
| return ( | ||
| a.S3Bucket === b.S3Bucket && | ||
| a.S3Region === b.S3Region && | ||
| a.S3CompatibleEndpoint === b.S3CompatibleEndpoint && | ||
| a.S3SecretRef.Name === b.S3SecretRef.Name && | ||
| (a.S3SecretRef.Namespace || '') === (b.S3SecretRef.Namespace || '') | ||
| ); | ||
| }; |
There was a problem hiding this comment.
use isEqual from lodash-es...
| const areS3ProfileFieldsEqual = ( | |
| a: S3StoreProfile, | |
| b: S3StoreProfile | |
| ): boolean => { | |
| return ( | |
| a.S3Bucket === b.S3Bucket && | |
| a.S3Region === b.S3Region && | |
| a.S3CompatibleEndpoint === b.S3CompatibleEndpoint && | |
| a.S3SecretRef.Name === b.S3SecretRef.Name && | |
| (a.S3SecretRef.Namespace || '') === (b.S3SecretRef.Namespace || '') | |
| ); | |
| }; |
| <Alert | ||
| variant={AlertVariant.danger} | ||
| isInline | ||
| title="Error loading storage classes" |
| if (!loaded) return <Spinner size="md" />; | ||
|
|
||
| if (error) { | ||
| return ( | ||
| <Alert | ||
| variant={AlertVariant.danger} | ||
| isInline | ||
| title="Error loading storage classes" | ||
| /> | ||
| ); | ||
| } |
There was a problem hiding this comment.
nits (optional): use StatusBox...
| <Alert | ||
| className={className} | ||
| variant={AlertVariant.warning} | ||
| title="Before you use Third-party storage" |
| actionLinks={ | ||
| <AlertActionLink | ||
| component="a" | ||
| href={docHref} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| > | ||
| View documentation | ||
| </AlertActionLink> | ||
| } |
There was a problem hiding this comment.
we have ViewDocumentation and ExternalLink, plz use any one of them...
| Ensure the storage backend supports replication. Carefully validate the | ||
| configured replication storage class supports failover, replication, and | ||
| recovery and then proceed. |
| const [show2, setShow2] = React.useState(false); | ||
|
|
||
| const [errors1, setErrors1] = React.useState< | ||
| Partial<Record<keyof S3Details, string>> |
There was a problem hiding this comment.
| Partial<Record<keyof S3Details, string>> | |
| Partial<S3Details> |
Wouldn't this be the same?
| Partial<Record<keyof S3Details, string>> | ||
| >({}); | ||
| const [errors2, setErrors2] = React.useState< | ||
| Partial<Record<keyof S3Details, string>> |
There was a problem hiding this comment.
| Partial<Record<keyof S3Details, string>> | |
| Partial<S3Details> |
same?
| Partial<Record<keyof S3Details, string>> | ||
| >({}); | ||
|
|
||
| const name1 = selectedClusters[0]?.metadata?.name || 'cluster-1'; |
There was a problem hiding this comment.
| const name1 = selectedClusters[0]?.metadata?.name || 'cluster-1'; | |
| const name1 = getName(selectedClusters[0]) || 'cluster-1'; |
| >({}); | ||
|
|
||
| const name1 = selectedClusters[0]?.metadata?.name || 'cluster-1'; | ||
| const name2 = selectedClusters[1]?.metadata?.name || 'cluster-2'; |
There was a problem hiding this comment.
| const name2 = selectedClusters[1]?.metadata?.name || 'cluster-2'; | |
| const name2 = getName(selectedClusters[1]) || 'cluster-2'; |
|
|
||
| const validate = (cluster: 1 | 2, details: S3Details) => { | ||
| const errs: Partial<Record<keyof S3Details, string>> = {}; | ||
| if (!details.bucketName.trim()) errs.bucketName = 'Bucket name is required'; |
There was a problem hiding this comment.
Translations for all these texts.
| const inputPadding = 'pf-v5-u-p-sm pf-v5-u-w-75'; | ||
|
|
||
| return ( | ||
| <div className="pf-v5-u-p-md"> |
There was a problem hiding this comment.
Either use form here or let the parent where this is used be a form with limitedWidth. forms have limited width prop.
| return ( | ||
| <div className="pf-v5-u-p-md"> | ||
| <ExpandableSection | ||
| toggleText={`S3 bucket for ${name1}`} |
| className={inputPadding} | ||
| > | ||
| {key === 'secretKey' ? ( | ||
| <div style={{ display: 'flex', alignItems: 'center' }}> |
Design: https://www.figma.com/design/ph6iiSTQD6bxEyw0rg0x3N/DR-for-Non--ODF-cluster--4.20-?node-id=6430-10848&t=kTN1qzE2epPNCiEr-0