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

add golangci config to project #194

Merged
merged 1 commit into from
Aug 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 14 additions & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# golangci-lint configuration file
# see: https://golangci-lint.run/usage/configuration/

# Options for analysis running
run:
# Which dirs to skip: they won't be analyzed;
skip-dirs:
- bin

# Settings for enabling and disabling linters
linters:
enable:
- goimports
- unparam
4 changes: 2 additions & 2 deletions pkg/utils/accelerators/tpu.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func getContainerRequestingTPUs(spec *corev1.PodSpec) *corev1.Container {
return nil
}

func addTPUVariablesSubGroup(pod *corev1.Pod, size int) error {
func addTPUVariablesSubGroup(pod *corev1.Pod) error {
container := getContainerRequestingTPUs(&pod.Spec)
if container == nil {
return nil
Expand Down Expand Up @@ -163,7 +163,7 @@ func addTPUVariablesSubGroup(pod *corev1.Pod, size int) error {
func AddTPUVariables(pod *corev1.Pod, size int) error {
_, foundSubGroupSize := pod.Annotations[leaderworkerset.SubGroupSizeAnnotationKey]
if foundSubGroupSize {
return addTPUVariablesSubGroup(pod, size)
return addTPUVariablesSubGroup(pod)
}
container := getContainerRequestingTPUs(&pod.Spec)
if container == nil {
Expand Down
6 changes: 1 addition & 5 deletions pkg/utils/accelerators/tpu_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ func TestAddTPUVariablesSubGroup(t *testing.T) {
tests := []struct {
name string
pod *corev1.Pod
size int
expectedTpuWorkerHostNames string
expectedTpuWorkerId string
}{
Expand All @@ -125,7 +124,6 @@ func TestAddTPUVariablesSubGroup(t *testing.T) {
},
},
},
size: 5,
expectedTpuWorkerId: "3",
expectedTpuWorkerHostNames: "test-sample-1.default,test-sample-1-1.default,test-sample-1-2.default,test-sample-1-3.default,test-sample-1-4.default",
},
Expand All @@ -146,7 +144,6 @@ func TestAddTPUVariablesSubGroup(t *testing.T) {
},
},
},
size: 8,
expectedTpuWorkerId: "3",
expectedTpuWorkerHostNames: "test-sample-1-4.default,test-sample-1-5.default,test-sample-1-6.default,test-sample-1-7.default",
},
Expand All @@ -166,14 +163,13 @@ func TestAddTPUVariablesSubGroup(t *testing.T) {
},
},
},
size: 9,
expectedTpuWorkerId: "0",
expectedTpuWorkerHostNames: "test-sample-1-5.default,test-sample-1-6.default,test-sample-1-7.default,test-sample-1-8.default",
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
err := addTPUVariablesSubGroup(tc.pod, tc.size)
err := addTPUVariablesSubGroup(tc.pod)
if err != nil {
t.Errorf("Error parsing parent: %s", err.Error())
}
Expand Down
12 changes: 6 additions & 6 deletions pkg/webhooks/leaderworkerset_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,13 @@ var _ webhook.CustomValidator = &LeaderWorkerSetWebhook{}

// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
func (r *LeaderWorkerSetWebhook) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
warnings, allErrs := r.generalValidate(obj)
return warnings, allErrs.ToAggregate()
allErrs := r.generalValidate(obj)
return nil, allErrs.ToAggregate()
}

// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
func (r *LeaderWorkerSetWebhook) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) {
warnings, allErrs := r.generalValidate(newObj)
allErrs := r.generalValidate(newObj)
specPath := field.NewPath("spec")

oldLws := oldObj.(*v1.LeaderWorkerSet)
Expand All @@ -97,15 +97,15 @@ func (r *LeaderWorkerSetWebhook) ValidateUpdate(ctx context.Context, oldObj, new
if newLws.Spec.LeaderWorkerTemplate.SubGroupPolicy == nil && oldLws.Spec.LeaderWorkerTemplate.SubGroupPolicy != nil {
allErrs = append(allErrs, field.Invalid(specPath.Child("leaderWorkerTemplate", "SubGroupPolicy", "subGroupSize"), oldLws.Spec.LeaderWorkerTemplate.SubGroupPolicy.SubGroupSize, "cannot remove subGroupSize after enabled"))
}
return warnings, allErrs.ToAggregate()
return nil, allErrs.ToAggregate()
}

// ValidateDelete implements webhook.Validator so a webhook will be registered for the type
func (r *LeaderWorkerSetWebhook) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
return nil, nil
}

func (r *LeaderWorkerSetWebhook) generalValidate(obj runtime.Object) (admission.Warnings, field.ErrorList) {
func (r *LeaderWorkerSetWebhook) generalValidate(obj runtime.Object) field.ErrorList {
lws := obj.(*v1.LeaderWorkerSet)
specPath := field.NewPath("spec")
metadataPath := field.NewPath("metadata")
Expand Down Expand Up @@ -157,7 +157,7 @@ func (r *LeaderWorkerSetWebhook) generalValidate(obj runtime.Object) (admission.
}
}

return nil, allErrs
return allErrs
}

// This is mostly inspired by https://github.com/kubernetes/kubernetes/blob/be4b7176dc131ea842cab6882cd4a06dbfeed12a/pkg/apis/apps/validation/validation.go#L460,
Expand Down