From a24560bd020b7d3fd8726574d797bbd7183fca93 Mon Sep 17 00:00:00 2001 From: Gondermann Date: Tue, 20 Jan 2026 16:11:58 +0100 Subject: [PATCH] Fix SecurityGroup availability status by counting security group rules SecurityGroups now count their specified rules and compare them to the number of rules in their ORC status and the openstack resource. As the security group rules are only ever part of one security group, this should be enough to reliably determine if all rules have been successfully created. On-behalf-of: SAP nils.gondermann@sap.com --- .../controllers/securitygroup/actuator.go | 9 ++++++--- internal/controllers/securitygroup/status.go | 19 ++++++++++++++++++- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/internal/controllers/securitygroup/actuator.go b/internal/controllers/securitygroup/actuator.go index 703f25c7c..09dbc5557 100644 --- a/internal/controllers/securitygroup/actuator.go +++ b/internal/controllers/securitygroup/actuator.go @@ -21,6 +21,7 @@ import ( "errors" "fmt" "iter" + "time" "github.com/gophercloud/gophercloud/v2/openstack/networking/v2/extensions/security/groups" "github.com/gophercloud/gophercloud/v2/openstack/networking/v2/extensions/security/rules" @@ -50,6 +51,11 @@ type ( securityGroupIterator = iter.Seq2[*osResourceT, error] ) +const ( + // The frequency to poll when waiting for the resource to become available + securityGroupAvailablePollingPeriod = 15 * time.Second +) + type securityGroupActuator struct { osClient osclients.NetworkClient k8sClient client.Client @@ -134,9 +140,6 @@ func (actuator securityGroupActuator) CreateResource(ctx context.Context, obj *o ProjectID: projectID, } - // FIXME(mandre) The security group inherits the default security group - // rules. This could be a problem when we implement `update` if ORC - // does not takes these rules into account. osResource, err := actuator.osClient.CreateSecGroup(ctx, &createOpts) if err != nil { // We should require the spec to be updated before retrying a create which returned a conflict diff --git a/internal/controllers/securitygroup/status.go b/internal/controllers/securitygroup/status.go index 94a83c8f7..90e172d45 100644 --- a/internal/controllers/securitygroup/status.go +++ b/internal/controllers/securitygroup/status.go @@ -45,7 +45,24 @@ func (securityGroupStatusWriter) ResourceAvailableStatus(orcObject orcObjectPT, } } - // SecurityGroup is available as soon as it exists + resourceSpec := orcObject.Spec.Resource + if resourceSpec != nil && resourceSpec.Rules != nil { + // Make sure specified security group rules exist in resource + + resourceStatus := orcObject.Status.Resource + if resourceStatus == nil || resourceStatus.Rules == nil { + return metav1.ConditionFalse, progress.WaitingOnOpenStack(progress.WaitingOnReady, securityGroupAvailablePollingPeriod) + } + + if len(resourceSpec.Rules) != len(resourceStatus.Rules) { + return metav1.ConditionFalse, progress.WaitingOnOpenStack(progress.WaitingOnReady, securityGroupAvailablePollingPeriod) + } + + if len(resourceSpec.Rules) != len(osResource.Rules) { + return metav1.ConditionFalse, progress.WaitingOnOpenStack(progress.WaitingOnReady, securityGroupAvailablePollingPeriod) + } + } + return metav1.ConditionTrue, nil }