feat: Implement SpaceBindingRequest controller main logic #372
Conversation
mfrancisc
requested review from
sbryzak,
xcoulon,
alexeykazakov,
MatousJobanek,
rajivnathan and
ranakan19
|
Be consistent with the rest of the constants format add consider adding comments. e.g. // SpaceBindingTerminatingReason represents the reason for space binding termination.
SpaceBindingTerminatingReason = terminatingReason
// SpaceBindingTerminatingFailedReason represents the reason for a failed space binding termination.
SpaceBindingTerminatingFailedReason = terminatingFailedReason
// SpaceBindingProvisioningFailedReason represents the reason for a failed space binding provisioning.
SpaceBindingProvisioningFailedReason = "UnableToProvision"
// SpaceBindingProvisioningReason represents the reason for space binding provisioning.
SpaceBindingProvisioningReason = provisioningReason
// SpaceBindingProvisionedReason represents the reason for a successfully provisioned space binding.
SpaceBindingProvisionedReason = provisionedReason |
api/v1alpha1/spacebinding_types.go
Outdated
| // --- Status condition reasons --- | ||
|
|
||
| // SpaceBindingTerminatingReason represents the reason for space binding termination. | ||
| SpaceBindingTerminatingReason = terminatingReason | ||
|
|
||
| // SpaceBindingTerminatingFailedReason represents the reason for a failed space binding termination. | ||
| SpaceBindingTerminatingFailedReason = terminatingFailedReason | ||
|
|
||
| // SpaceBindingProvisioningFailedReason represents the reason for a failed space binding provisioning. | ||
| SpaceBindingProvisioningFailedReason = "UnableToProvision" | ||
|
|
||
| // SpaceBindingProvisioningReason represents the reason for space binding provisioning. | ||
| SpaceBindingProvisioningReason = provisioningReason | ||
|
|
||
| // SpaceBindingProvisionedReason represents the reason for a successfully provisioned space binding. | ||
| SpaceBindingProvisionedReason = provisionedReason |
There was a problem hiding this comment.
shouldn't be these constant placed rather in SpaceBindingRequest file?
and also, since they are going to be used in SBR CR only, then we should name them accordingly.
There was a problem hiding this comment.
Similar usage in the UserSignup case: https://github.com/codeready-toolchain/api/blob/master/api/v1alpha1/usersignup_types.go#L80
We could also rename UnableToProvision to UnableToCreateSpaceBinding
There was a problem hiding this comment.
@MatousJobanek thanks for pointing out the usersignup example!
The reason I've added them in the spacebinding_types and kept this naming was mainly because I was following the SpaceRequest example, where we reflect the Ready condition from the Space object to the SpaceRequest. Similarly, as pointed out in one of our Slack discussions, I expected those conditions to be available in the SpaceBinding.Status.Conditions and be able to copy them in the SpaceBindingRequest.Status.Conditions. For the time being I've added them here with the plan of implementing conditions in the SpaceBinding resource , as we discussed. But I don't have a strong opinion on this, let's just decide which example we want to follow (the UserSignup one or the SpaceRequest example).
There was a problem hiding this comment.
@MatousJobanek as discussed in our 1:1 today I've updated the PR and moved the reasons to spacebindingrequest_types.go , also I've renamed them accordingly. Please take a look at fac7152 when you have some time and thanks for your pointers!
|
Kudos, SonarCloud Quality Gate passed!
|
Description
Add SpaceBinding provisioning reasons. Those will be used by the SpaceBindingRequest controller in the conditions of the SBR objects.
Jira: https://issues.redhat.com/browse/ASC-397
Checks
Did you run
make generatetarget? yesDid
make generatechange anything in other projects (host-operator, member-operator)? noIn case of new CRD, did you the following? N/A
In case other projects are changed, please provides PR links.