Skip to content

Conversation

@mivanov1988
Copy link
Contributor

@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
Contributor

@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.

5 participants