-
Notifications
You must be signed in to change notification settings - Fork 667
Hide linux bridge option if HCO is not installed #3183
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
Hide linux bridge option if HCO is not installed #3183
Conversation
...ugin/src/components/network-attachment-definitions/NetworkAttachmentDefinitionCreateYaml.tsx
Outdated
Show resolved
Hide resolved
...on-plugin/src/components/network-attachment-definitions/NetworkAttachmentDefinitionsForm.tsx
Outdated
Show resolved
Hide resolved
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.
missing types
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.
why changing this to any ?
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.
The NetworkAttachmentDefinitionConfig type didn't really fit anymore. I added more specific types. I'm planning on a follow-up to increase the type support throughout the plugin, as well.
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.
please create enum out of these keys so you dont have to use strings in the rest of the code
...on-plugin/src/components/network-attachment-definitions/NetworkAttachmentDefinitionsForm.tsx
Outdated
Show resolved
Hide resolved
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.
dont we want to create model for HyperConverged CRD ?
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.
We're not actually using the HyperConverged CRD aside from testing that it exists, so I wasn't sure if I should create one. I went ahead and made it. It's easy enough to remove if we decide it's unnecessary.
ad28334 to
2bc7c67
Compare
2bc7c67 to
d840b65
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: pcbailey, rawagner 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 |
|
/retest |
This PR hides the "Linux bridge" Network Type option in the network attachment definitions creation form if the HyperConverged Cluster Operator isn't installed. It also disables the Network Type dropdown if no types are available.
@phoracek