Conversation
|
/retest |
eshulman2
force-pushed
the
add_lb
branch
8 times, most recently
from
d616eb6 to
4a22540
Compare
- allow testing octavia by enabling it in the e2e job - increase vm qouta to allow e2e to run
$ go run ./cmd/scaffold-controller -interactive=false -kind=LoadBalancer -gophercloud-client=NewLoadBalancerV2 -gophercloud-module=github.com/gophercloud/gophercloud/v2/openstack/loadbalancer/v2/loadbalancers -optional-create-dependency Subnet -optional-create-dependency Network -optional-create-dependency Port -optional-create-dependency Flavor -optional-create-dependency Project
Add support for OpenStack Octavia Load Balancer resources. This includes: - LoadBalancer CRD with support for VIP subnet/network/port references - Controller with create, update, delete, and import capabilities - Status reporting with provisioning and operating status - Dependency resolution for Subnet, Network, Port, and Project references - Kuttl tests for create, update, import, and dependency scenarios Closes k-orc#619
winiciusallan
left a comment
winiciusallan
left a comment
There was a problem hiding this comment.
Hi @eshulman2, that was a huge job, huh? I've just made a small round of review, and I'll take a look at the missing files this week.
Let me know what you think about the comments.
| // vipSubnetRef is the subnet on which to allocate the load balancer's address. | ||
| // +optional | ||
| // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="vipSubnetRef is immutable" | ||
| VipSubnetRef *KubernetesNameRef `json:"vipSubnetRef,omitempty"` |
There was a problem hiding this comment.
nit: since VIP is an acronym, I would change it to VIPSubnetRef. This counts for all occurrences.
K8s has a convention for this 👉 https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#constants
| return progress.WrapError(err) | ||
| } | ||
|
|
||
| func (actuator loadbalancerActuator) updateResource(ctx context.Context, obj orcObjectPT, osResource *osResourceT) progress.ReconcileStatus { |
There was a problem hiding this comment.
I'm wondering if we need to check for the Loadbalancer status before performing an update operation, since we can't update the load balancer if it has a PENDING_UPDATE status. I think the controller itself will retry the update when checking the NeedsRefresh. Am I missing something?
| metadata: | ||
| name: loadbalancer-create-minimal | ||
| status: | ||
| resource: |
There was a problem hiding this comment.
I would add adminStateUp here as well since it has a default value.
| kind: Subnet | ||
| name: loadbalancer-create-minimal | ||
| ref: subnet | ||
| assertAll: |
There was a problem hiding this comment.
You could exercise more dynamic checks here, like having an empty description. Is there a default availability zone?
| [[post-config|/etc/nova/nova.conf]] | ||
| [quota] | ||
| instances = 50 |
There was a problem hiding this comment.
I didn't get why we need to change the instance quota here. Could you please explain?
There was a problem hiding this comment.
Should this file be here or is it part of a new enhancement proposal?
2 participants
Add support for OpenStack Octavia Load Balancer resources. This includes: