Skip to content

Add default comment and defaulting logic for TaskSpec.MinAvailable #188

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add default comment and defaulting logic for TaskSpec.MinAvailable #188

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions pkg/apis/batch/v1alpha1/job.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,13 @@ type TaskSpec struct {
DependsOn *DependsOn `json:"dependsOn,omitempty" protobuf:"bytes,8,opt,name=dependsOn"`
}

// Default sets default values for TaskSpec fields.
func (t *TaskSpec) Default() {
Copy link
Member

Choose a reason for hiding this comment

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

Can this take effect when the job CRD created?

Copy link
Author

Choose a reason for hiding this comment

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

The Default() method will only take effect if a defaulting webhook is enabled for the Job CRD, or if the API server is set up to call it via controller-runtime's defaulting registration. If neither is set up, the method will not be invoked automatically when a Job is created.

Copy link
Author

Choose a reason for hiding this comment

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

Can this take effect when the job CRD created?

sir waiting for the confirmation ?

Copy link
Member

Choose a reason for hiding this comment

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

Sorry for late.
In volcano, there is a seperate webhook,so I'm curious when and where this Default method will take effect: )

if t.MinAvailable == nil {
t.MinAvailable = &t.Replicas
Copy link

Choose a reason for hiding this comment

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

high

Assigning &t.Replicas directly to t.MinAvailable creates an alias between these two fields, which can cause unexpected side effects and inconsistent behavior after a DeepCopy. To avoid this, allocate a new int32 for MinAvailable and copy the value from Replicas.

v := t.Replicas
	t.MinAvailable = &v

}
}

// JobPhase defines the phase of the job.
type JobPhase string

Expand Down