-
Notifications
You must be signed in to change notification settings - Fork 3
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
Extend workload pools to include ingress firewall and ip allocations #6
Extend workload pools to include ingress firewall and ip allocations #6
Conversation
8b85a58
to
5b526af
Compare
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.
Just a couple tweaks that can improve things, but I'm not going to hold you up for now...
@@ -256,6 +424,38 @@ components: | |||
machine: | |||
flavorId: c7568e2d-f9ab-453d-9a3a-51375f78426b | |||
replicas: 3 | |||
firewall: |
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.
Examples are obviously the best illustration to review 😸
Looks good so far. I'd have a word with Adam, the firewall rules may need a description to make the UI happier.
publicIP: | ||
description: Machine public IP address. | ||
type: string | ||
status: |
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 reference https://github.com/unikorn-cloud/core/blob/main/pkg/openapi/common.spec.yaml#L90 here to keep things in sync.
port: | ||
$ref: '#/components/schemas/firewallRulePort' | ||
cidr: | ||
description: A list of CIDR blocks to allow, it might be any IPv4 or IPv6 in CIDR notation. |
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.
Surprise I've not done this yet, but we should probably have a type definition in core for an IPv4 CIDR with the required regular expression patter matcher too.
This PR extends workload pools to include ingress firewall and IP allocations and add a status so users can discover the hostname and IP addresses of provisioned machines