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: handle deployment deletion in case of a job being deleted #2816

Merged
merged 5 commits into from
Oct 19, 2023

Conversation

mivanov1988
Copy link
Collaborator

@mivanov1988 mivanov1988 commented Oct 19, 2023

Why

When the data job is deleted, both desired deployment records are deleted, but the actual deployment records and Kubernetes cron jobs are still present.

What

Extended the deployment synchronization to cover this scenario. Added an additional flyway script as part of the integration tests because of the different H2 constraint naming.

Testing done

Integration tests.

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

@mivanov1988 mivanov1988 changed the title Person/miroslavi/improve async deployment deletion control-service: handle deployment deletion in case of a job being deleted Oct 19, 2023
@mivanov1988 mivanov1988 force-pushed the person/miroslavi/improve-async-deployment-deletion branch from 6757a79 to 2ce3965 Compare October 19, 2023 11:06
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

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.

Looks good to me. Thanks for addressing those changes.

Please make sure vdk-heartbeat passes reliably (3 times in sequence for example).

I am concerned that changing from syncrhouos to more async/eventual consistence model might introduce some intermittent issues so do not rely on passing immediately and monitor the control service pipeline a few days after the merge.

@mivanov1988 mivanov1988 enabled auto-merge (squash) October 19, 2023 14:15
@mivanov1988 mivanov1988 force-pushed the person/miroslavi/improve-async-deployment-deletion branch from 0a7181f to 2ce3965 Compare October 19, 2023 14:22
mivanov1988 and others added 2 commits October 19, 2023 17:23
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 aivanov's comment. You can find the discussion on this change in the following link: Link to the GitHub pull request discussion

Signed-off-by: Miroslav Ivanov miroslavi@vmware.com
@mivanov1988 mivanov1988 force-pushed the person/miroslavi/improve-async-deployment-deletion branch from a758c55 to 9c8baa8 Compare October 19, 2023 14:27
…leted

Why
When the data job is deleted, both desired deployment records are deleted, but the actual deployment records and Kubernetes cron jobs are still present.

What
Extended the deployment synchronization to cover this scenario.

Testing done
Integration tests.

Signed-off-by: Miroslav Ivanov miroslavi@vmware.com
@mivanov1988 mivanov1988 force-pushed the person/miroslavi/improve-async-deployment-deletion branch from 5109362 to 5d54cb5 Compare October 19, 2023 15:16
@mivanov1988 mivanov1988 merged commit cab16e7 into main Oct 19, 2023
@mivanov1988 mivanov1988 deleted the person/miroslavi/improve-async-deployment-deletion branch October 19, 2023 16:13
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.

4 participants