Skip to content
This repository has been archived by the owner on Sep 12, 2023. It is now read-only.

feat(common): support delay pods creation #130

Merged
merged 2 commits into from
Jul 26, 2021

Conversation

shinytang6
Copy link
Member

@shinytang6 shinytang6 commented Apr 6, 2021

resolves #129


if pg == nil || pg.Status.Phase == "" || pg.Status.Phase == v1beta1.PodGroupPending {
log.Warnf("PodGroup %v unschedulable", jobKey)
return nil
Copy link
Member

Choose a reason for hiding this comment

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

Did we watch PodGroup?

I am not sure if there is a new reconciliation loop for the status change.

Copy link
Member Author

@shinytang6 shinytang6 Apr 6, 2021

Choose a reason for hiding this comment

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

seems not, l think we should support it(watching podgroup) in tfoperator?

return nil
}

if pg == nil || pg.Status.Phase == "" || pg.Status.Phase == v1beta1.PodGroupPending {
Copy link
Member

Choose a reason for hiding this comment

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

Could you add some code comments on this, similar to issue description in #129?

Copy link
Member Author

Choose a reason for hiding this comment

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

done, PTAL

@shinytang6 shinytang6 force-pushed the feat/delay-pod-creation branch 2 times, most recently from d595ab0 to c6fc9ab Compare April 6, 2021 16:06
now := metav1.Now()
jobStatus.LastReconcileTime = &now

return jc.Controller.UpdateJobStatusInApiServer(job, &jobStatus)
Copy link
Member

Choose a reason for hiding this comment

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

I think we could add some comments here to let us know that we do this update to trigger another reconcile. And maybe in the future the operator should watch PodGroup directly.

@gaocegege
Copy link
Member

/assign @terrytangyuan

/LGTM

Copy link
Member

@terrytangyuan terrytangyuan left a comment

Choose a reason for hiding this comment

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

Thanks!

/lgtm
/approve

@google-oss-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: terrytangyuan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-robot google-oss-robot merged commit 5dabcf9 into kubeflow:master Jul 26, 2021
georgkaleido pushed a commit to georgkaleido/common that referenced this pull request Jun 9, 2022
Co-authored-by: depfu[bot] <23717796+depfu[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature] Support delay pods creating when EnableGangScheduling is true
4 participants