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

Feat: delete the sync deployment after deploy complete #78

Merged
merged 5 commits into from
Sep 12, 2023

Conversation

lizonglingo
Copy link
Contributor

This pr resolves #70 .

Closes #70 .

logger.Debugf("node count is %d", nodeCount)
}

checkTicker := time.NewTicker(30 * time.Second)
Copy link
Collaborator

@caoxianfei1 caoxianfei1 Aug 11, 2023

Choose a reason for hiding this comment

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

The for{} never stop if something wrong, you can add a timeout time or limit retry times.

logger.Infof("cluster is deployed, deployment about \"curve-sync-config\" will be deleted")
}

func isAllReplicasReady(deployment apps.Deployment) bool {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The deployment operation and job operation code put the k8sutil dir is better.

@@ -86,6 +98,10 @@ func createSyncDeployment(c *daemon.Cluster) error {
return err
}
}

// delete the SyncConfigDeployment after the cluster is deployed.
Copy link
Collaborator

Choose a reason for hiding this comment

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

using channel to get delete result (true or false) and log the reason if delete the deployment failed.

}

func isJobCompleted(job batch.Job) bool {
if *job.Spec.Completions == job.Status.Succeeded {
Copy link
Collaborator

Choose a reason for hiding this comment

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

the job status is failed or other status sometimes.

@lizonglingo
Copy link
Contributor Author

ok, I'll make some changes and additions @caoxianfei1

@caoxianfei1
Copy link
Collaborator

ok, I'll make some changes and additions @caoxianfei1

ok

@lizonglingo
Copy link
Contributor Author

new code has been submitted @caoxianfei1

@caoxianfei1 caoxianfei1 self-requested a review August 25, 2023 09:48
@@ -154,3 +157,22 @@ func getReadConfigJobLabel(c *daemon.Cluster) map[string]string {
labels["curve"] = c.Kind
return labels
}

// deleteSyncConfigDeployment delete the SyncConfigDeployment after the cluster is deployed.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that put the func in k8sutil fold and deployment file is better

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, I've adjusted its directory

for func DeleteSyncConfigDeployment()
@caoxianfei1
Copy link
Collaborator

lgtm

@caoxianfei1 caoxianfei1 merged commit 9bbfa61 into opencurve:master Sep 12, 2023
1 check passed
caoxianfei1 pushed a commit that referenced this pull request Sep 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Delete the sync Deployment after deploy complete.
2 participants