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

control-service: asynchronous deployment deletion #2781

Merged

Conversation

mivanov1988
Copy link
Contributor

Why:
As part of VEP-2272, we need to introduce a process for synchronizing data jobs from the database to Kubernetes. Currently, the process lacks the ability to delete deployments asynchronously.

What:
We have introduced an asynchronous method for deployment deletion. It is based on the logic that if the desired deployment is null and the actual deployment is not null, the process performs deployment deletion.

Testing done:
Integration tests.

Signed-off-by: Miroslav Ivanov miroslavi@vmware.com

@mivanov1988 mivanov1988 force-pushed the person/miroslavi/utilize-deployment-persistence-flags branch from 5f065db to 1f95184 Compare October 10, 2023 13:13
@mivanov1988 mivanov1988 enabled auto-merge (squash) October 10, 2023 13:34
@mivanov1988 mivanov1988 force-pushed the person/miroslavi/utilize-deployment-persistence-flags branch from 7921b66 to 1f9cb00 Compare October 11, 2023 14:21
Copy link
Collaborator

@dakodakov dakodakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mivanov1988 mivanov1988 force-pushed the person/miroslavi/utilize-deployment-persistence-flags branch from 6c3118c to bc6c3dc Compare October 12, 2023 08:25
Copy link
Collaborator

@antoniivanov antoniivanov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if data job is deleted?

@mivanov1988
Copy link
Contributor Author

What happens if data job is deleted?

That is a very good point! When you delete a data job, the database will automatically remove the associated deployments from the desired and actual tables. Additionally, the old DeploymentService.deleteDeployment() will be called, and the K8s artifacts will be deleted as well. However, because the synchronization process caches the data jobs and deployments, if a user deletes a data job during synchronization, the Kubernetes artifacts will be left in the cluster, potentially leading to an inconsistent state.

In my opinion, we should remove the references from the desired and actual deployment tables to the data job table. In this scenario, we can delete the data job record from the 'data_job' table and the deployment record from the 'desired_data_job_deployment' table. Subsequently, we will have an existing deployment record in the 'actual_data_job_deployment' table, which will be deleted by the synchronization process during the next synchronization, along with all K8S-related artifacts.

@antoniivanov what do you think?

@antoniivanov
Copy link
Collaborator

What happens if data job is deleted?

That is a very good point! When you delete a data job, the database will automatically remove the associated deployments from the desired and actual tables. Additionally, the old DeploymentService.deleteDeployment() will be called, and the K8s artifacts will be deleted as well. However, because the synchronization process caches the data jobs and deployments, if a user deletes a data job during synchronization, the Kubernetes artifacts will be left in the cluster, potentially leading to an inconsistent state.

In my opinion, we should remove the references from the desired and actual deployment tables to the data job table. In this scenario, we can delete the data job record from the 'data_job' table and the deployment record from the 'desired_data_job_deployment' table. Subsequently, we will have an existing deployment record in the 'actual_data_job_deployment' table, which will be deleted by the synchronization process during the next synchronization, along with all K8S-related artifacts.

@antoniivanov what do you think?

Seems a good idea. It make sense that user should not be able to directly touch actual_deployment , this should be done only by the automation. Users (aka API endpoints) interact only with desired state

We need to make sure to test some corner cases:

  • synchronization process is delayed or fails (pod restarts) for some reason after the data job is deleted
  • job is created almost immediately with the same name after starting deletion
  • What would the GraphQL endpoint return (aka the UI show) Probably the job as deleted and hence nothing

@mivanov1988 mivanov1988 force-pushed the person/miroslavi/utilize-deployment-persistence-flags branch from be71354 to 32835d1 Compare October 13, 2023 09:09
@mivanov1988
Copy link
Contributor Author

mivanov1988 commented Oct 13, 2023

What happens if data job is deleted?

That is a very good point! When you delete a data job, the database will automatically remove the associated deployments from the desired and actual tables. Additionally, the old DeploymentService.deleteDeployment() will be called, and the K8s artifacts will be deleted as well. However, because the synchronization process caches the data jobs and deployments, if a user deletes a data job during synchronization, the Kubernetes artifacts will be left in the cluster, potentially leading to an inconsistent state.
In my opinion, we should remove the references from the desired and actual deployment tables to the data job table. In this scenario, we can delete the data job record from the 'data_job' table and the deployment record from the 'desired_data_job_deployment' table. Subsequently, we will have an existing deployment record in the 'actual_data_job_deployment' table, which will be deleted by the synchronization process during the next synchronization, along with all K8S-related artifacts.
@antoniivanov what do you think?

Seems a good idea. It make sense that user should not be able to directly touch actual_deployment , this should be done only by the automation. Users (aka API endpoints) interact only with desired state

We need to make sure to test some corner cases:

  • synchronization process is delayed or fails (pod restarts) for some reason after the data job is deleted

It will delete the cron job on the next synchronization.

  • job is created almost immediately with the same name after starting deletion

It is okay. The cron job will be deleted, and the job will no longer have an associated one. The UI might display the existing deployment for a short period of time.

  • What would the GraphQL endpoint return (aka the UI show) Probably the job as deleted and hence nothing

Nothing happens because the job is a first-class citizen.

@mivanov1988 mivanov1988 force-pushed the person/miroslavi/utilize-deployment-persistence-flags branch from 9ea1a95 to 2f3f63b Compare October 13, 2023 09:32
Why:
As part of VEP-2272, we need to introduce a process for synchronizing data jobs from the database to Kubernetes.
Currently, the process lacks the ability to delete deployments asynchronously.

What:
We have introduced an asynchronous method for deployment deletion. It is based on the logic that
if the desired deployment is null and the actual deployment is not null, the process performs deployment deletion.

Testing done:
Integration tests.

Signed-off-by: Miroslav Ivanov miroslavi@vmware.com
@mivanov1988 mivanov1988 force-pushed the person/miroslavi/utilize-deployment-persistence-flags branch from 33e5bfb to 9f44b2d Compare October 13, 2023 09:45
@mivanov1988 mivanov1988 merged commit 0e92579 into main Oct 13, 2023
9 of 10 checks passed
@mivanov1988 mivanov1988 deleted the person/miroslavi/utilize-deployment-persistence-flags branch October 13, 2023 11:46
mivanov1988 added a commit that referenced this pull request Oct 19, 2023
I've introduced the initial version of async job deployment integration
tests. This is just the starting point, and we plan to expand and
refactor it further in the upcoming version based on feedback from
@antoniivanov 's comment. You can find the discussion on this change in
the following link: [Link to the GitHub pull request
discussion](#2781 (comment)).

Signed-off-by: Miroslav Ivanov miroslavi@vmware.com

---------

Signed-off-by: Miroslav Ivanov miroslavi@vmware.com
Co-authored-by: github-actions <>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants