Skip to content
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

Review the claiming code logic #313

Open
cmoulliard opened this issue May 8, 2023 · 5 comments
Open

Review the claiming code logic #313

cmoulliard opened this issue May 8, 2023 · 5 comments
Labels
enhancement New feature or request

Comments

@cmoulliard
Copy link
Contributor

cmoulliard commented May 8, 2023

TODO

Review the claiming code logic when the service can be discovered or is installable (using crossplane helm releaseby example) on a k8s cluster.

The existing code do not seem to follow this flow here and here

image

@cmoulliard cmoulliard added the enhancement New feature or request label May 8, 2023
@cmoulliard
Copy link
Contributor Author

cmoulliard commented May 9, 2023

Ideally the flow should be implemented part of a Service (or job running every x seconds) to check about the new claims CRs or Claim sql records and then starting to process them when status is equal to New.

As soon as a claim is processed, then the job/service change the status from New to Pending to indicate that it is currently processed and to avoid that it is processed twice, etc. The status Pending indicates also that the claim is locked till the service will go through the different steps to reach the end (= bound) or to fail (error) for different reasons

aureamunoz added a commit to aureamunoz/servicebox-poc that referenced this issue May 18, 2023
@aureamunoz
Copy link
Contributor

I think we should review the architecture code in order to move the business logic to the service layer. Currently we have methods called doXXXX in the Resource classes containing business logic (for example, binding claims or creating K8s resources). IMO we should move this logic to the corresponding service class. I would also do the persist tasks in the service layer.

Regarding the claiming, if we let only an entry point to the claiming action from the Applications UI, why would we need a job? Shouldn't we just create the claim + bind the application if all needed elements exist?
WDYT @cmoulliard @Sgitario ?

@Sgitario
Copy link
Collaborator

I think we should review the architecture code in order to move the business logic to the service layer. Currently we have methods called doXXXX in the Resource classes containing business logic (for example, binding claims or creating K8s resources). IMO we should move this logic to the corresponding service class. I would also do the persist tasks in the service layer.

+100

Regarding the claiming, if we let only an entry point to the claiming action from the Applications UI, why would we need a job? Shouldn't we just create the claim + bind the application if all needed elements exist? WDYT @cmoulliard @Sgitario ?

If this is not an automatic action (we don't need to discover the services or something like this), then we don't need a job. Though if we need to verify the claim status every X minutes (for example, to check whether the service still exists), then we would need a job to do this.

@cmoulliard
Copy link
Contributor Author

If this is not an automatic action (we don't need to discover the services or something like this), then we don't need a job.

Even if we dont need a job, we need one or some services able to handle the different steps of the flow. Having a job or service scheduler able to process the different steps allows to perform the work in an async mode

@aureamunoz
Copy link
Contributor

Ok, let's keep the scheduled job at the moment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants