🐛 Security group rule creation fails when rule already exists

[bnallapeta]

/kind bug

What steps did you take and what happened:
When creating security group rules in OpenStack, if a rule already exists, the operation fails with HTTP 409 error instead of being treated as idempotent.

What did you expect to happen:
Security group rule creation should be idempotent - if a rule already exists, it should be treated as a successful operation.

Have fixed it here:

cluster-api-provider-openstack/pkg/cloud/services/networking/securitygroups.go

Lines 568 to 572 in 0ab4e3c

	// Handle HTTP 409 (SecurityGroupRuleExists) as success - the rule already exists
	if strings.Contains(err.Error(), "SecurityGroupRuleExists") || strings.Contains(err.Error(), "already exists") {
	s.scope.Logger().V(4).Info("Security group rule already exists, treating as success", "description", r.Description, "securityGroupID", securityGroupID)
	return nil
	}

[bnallapeta]

@mdbooth Let me know what you think. I will work on a PR for this (along with unit tests of course :p)

[EmilienM]

Please explain how did you hit that bug first, thanks

[mdbooth]

This suggests that we're racing with something, so I'd also want to understand how this occurs. Nothing else should be creating this rule. Are you running multiple CAPO controllers concurrently, by any chance? If so that's going to have a variety of fun edge cases. Or perhaps we're calling this operation via some code path which can be invoked concurrently on the same resource? That would be a bug.

Incidentally, I'm generally not in favour or string matching error messages: developers rarely treat them like an API, so breakage can occur at any time.

My main suggestion here is try to understand how it is happening. It should not be happening. You should focus on fixing this first.

If we do decide that this race is legitimate and we need to handle it then the preferred behaviour for handling a 409 is to retry. In this case presumably just return an ephemeral error and wait to be reconciled again.