Display a warning if preserve static IPs are enabled with pod network mapping #1387
Conversation
sgratch
added
the
enhancement
sgratch
force-pushed
the
Warn-preserve-staticIP-mapped-to-pod-network
branch
from
6284b5d to
545b4b8
Compare
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #1387 +/- ##
==========================================
- Coverage 36.81% 36.21% -0.60%
==========================================
Files 158 159 +1
Lines 2548 2579 +31
Branches 599 603 +4
==========================================
- Hits 938 934 -4
- Misses 1428 1643 +215
+ Partials 182 2 -180 ☔ View full report in Codecov by Sentry. |
sgratch
changed the title
|
cc: @RichardHoch |
.../forklift-console-plugin/src/modules/Plans/views/details/components/PlanWarningCondition.tsx
Outdated
Show resolved
Hide resolved
|
|
||
| import { Alert, Text, TextContent, TextVariants } from '@patternfly/react-core'; | ||
|
|
||
| export const PlanWarningCondition: React.FC<{ |
There was a problem hiding this comment.
no need for React.FC , u can directly import FC
There was a problem hiding this comment.
More than half of our current code declare components like above, so if we want to use one format for all, I'll suggest to fix that in a follow up PR.
| }> = ({ type, message, suggestion }) => { | ||
| const { t } = useTranslation(); | ||
| return ( | ||
| <Alert title={t('The plan migration might not work as expected - ') + type} variant="warning"> |
There was a problem hiding this comment.
there is AlertVaraitn.warnning from patternfly, better use it then manually declaring
There was a problem hiding this comment.
by the way, better to include '-' in a trans string,
- polluting the trans JSON file, making it probably unsharable translation,
- a '-' char that has no meaning in translation.
There was a problem hiding this comment.
there is AlertVaraitn.warnning from patternfly, better use it then manually declaring
Please explain, not sure I understand this comment
by the way, better to include '-' in a trans string,
Just to make sure, you mean to exclude right? If so then sure. Fixed for all occurrences in the code.
There was a problem hiding this comment.
like there is a ButtonVariant u can import from patternfly, and use this enum for the variant prop value , there is also AlertVariant
yes I mean exclude. thanks.
There was a problem hiding this comment.
like there is a ButtonVariant u can import from patternfly, and use this enum for the variant prop value , there is also AlertVariant
The current code is already using the warning variant:
That's why I don't understand your suggestion. Can you please give an example?
There was a problem hiding this comment.
what i mean
import {ButtonVariant } pattern..
<Button variant={ButtonVariant.warning} >something</Button>
There was a problem hiding this comment.
Worth fixing it for all other code occurrences in a follow up PR.
There was a problem hiding this comment.
Sure please open tasks for the follow ups
There was a problem hiding this comment.
.../forklift-console-plugin/src/modules/Plans/views/details/components/PlanWarningCondition.tsx
Outdated
Show resolved
Hide resolved
...ages/forklift-console-plugin/src/modules/Plans/views/details/components/PlanPageHeadings.tsx
Show resolved
Hide resolved
...ages/forklift-console-plugin/src/modules/Plans/views/details/components/PlanPageHeadings.tsx
Outdated
Show resolved
Hide resolved
...ages/forklift-console-plugin/src/modules/Plans/views/details/components/PlanPageHeadings.tsx
Outdated
Show resolved
Hide resolved
sgratch
force-pushed
the
Warn-preserve-staticIP-mapped-to-pod-network
branch
from
545b4b8 to
ec5b7a1
Compare
sgratch
force-pushed
the
Warn-preserve-staticIP-mapped-to-pod-network
branch
from
ec5b7a1 to
3687da0
Compare
|
@metalice please review |
Reference:https://issues.redhat.com/browse/MTV-1503 In case there is no critical error for a plan and in addition, the plan is set to enable the preserve static IPs for the VMs while at least one of the destination network mappings is set to 'Pod Networking' type => then display a warning alert at the head of plan page. Signed-off-by: Sharon Gratch <[email protected]>
sgratch
force-pushed
the
Warn-preserve-staticIP-mapped-to-pod-network
branch
from
3687da0 to
e8f26a2
Compare
|
Reference:https://issues.redhat.com/browse/MTV-1503
In case there is no critical error for a plan and in addition, the plan is set to enable the preserve static IPs for the VMs while at least one of the destination network mappings is set to 'Pod Networking' type => then display a warning alert at the head of plan page.
Screenshots
Screencast.from.2024-11-13.21-35-45.webm