Skip to content

Commit

Permalink
[Style] Fix golangci-lint rule: revive (#2167)
Browse files Browse the repository at this point in the history
  • Loading branch information
MortalHappiness authored Jun 7, 2024
1 parent 8875c8b commit d149356
Show file tree
Hide file tree
Showing 20 changed files with 135 additions and 139 deletions.
11 changes: 10 additions & 1 deletion ray-operator/.golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ linters-settings:
- name: error-strings
- name: errorf
- name: exported
disabled: true
- name: if-return
- name: increment-decrement
- name: indent-error-flow
Expand All @@ -33,6 +34,14 @@ linters-settings:
- name: unused-parameter
- name: var-declaration
- name: var-naming
exclude:
- "**/ray-operator/apis/config/v1alpha1/*.go"
- "**/ray-operator/apis/ray/v1alpha1/*.go"
- "**/ray-operator/apis/ray/v1/*.go"
arguments:
- ["ID", "JSON", "HTTP", "IP"] # AllowList
- [] # DenyList
- - upperCaseConst: true
gocyclo:
min-complexity: 15
govet:
Expand Down Expand Up @@ -60,7 +69,7 @@ linters:
- nilerr
- noctx
- predeclared
# - revive
- revive
- staticcheck
- typecheck
- unconvert
Expand Down
2 changes: 1 addition & 1 deletion ray-operator/apis/ray/v1/raycluster_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func (r *RayCluster) ValidateCreate() (admission.Warnings, error) {
}

// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
func (r *RayCluster) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
func (r *RayCluster) ValidateUpdate(_ runtime.Object) (admission.Warnings, error) {
rayclusterlog.Info("validate update", "name", r.Name)
return nil, r.validateRayCluster()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,18 +52,18 @@ func (d *DefaultBatchScheduler) Name() string {
return GetDefaultPluginName()
}

func (d *DefaultBatchScheduler) DoBatchSchedulingOnSubmission(ctx context.Context, app *rayv1.RayCluster) error {
func (d *DefaultBatchScheduler) DoBatchSchedulingOnSubmission(_ context.Context, _ *rayv1.RayCluster) error {
return nil
}

func (d *DefaultBatchScheduler) AddMetadataToPod(app *rayv1.RayCluster, groupName string, pod *corev1.Pod) {
func (d *DefaultBatchScheduler) AddMetadataToPod(_ *rayv1.RayCluster, _ string, _ *corev1.Pod) {
}

func (df *DefaultBatchSchedulerFactory) New(config *rest.Config) (BatchScheduler, error) {
func (df *DefaultBatchSchedulerFactory) New(_ *rest.Config) (BatchScheduler, error) {
return &DefaultBatchScheduler{}, nil
}

func (df *DefaultBatchSchedulerFactory) AddToScheme(scheme *runtime.Scheme) {
func (df *DefaultBatchSchedulerFactory) AddToScheme(_ *runtime.Scheme) {
}

func (df *DefaultBatchSchedulerFactory) ConfigureReconciler(b *builder.Builder) *builder.Builder {
Expand Down
22 changes: 12 additions & 10 deletions ray-operator/controllers/ray/batchscheduler/schedulermanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,18 +72,20 @@ func (batch *SchedulerManager) GetScheduler(schedulerName string) (schedulerinte
batch.Lock()
defer batch.Unlock()

if plugin, existed := batch.plugins[schedulerName]; existed && plugin != nil {
plugin, existed := batch.plugins[schedulerName]

if existed && plugin != nil {
return plugin, nil
} else if existed && plugin == nil {
}
if existed && plugin == nil {
return nil, fmt.Errorf(
"failed to get scheduler plugin %s, previous initialization has failed", schedulerName)
} else {
if plugin, err := factory.New(batch.config); err != nil {
batch.plugins[schedulerName] = nil
return nil, err
} else {
batch.plugins[schedulerName] = plugin
return plugin, nil
}
}
plugin, err := factory.New(batch.config)
if err != nil {
batch.plugins[schedulerName] = nil
return nil, err
}
batch.plugins[schedulerName] = plugin
return plugin, nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,7 @@ func (v *VolcanoBatchScheduler) DoBatchSchedulingOnSubmission(ctx context.Contex
totalResource = utils.CalculateMinResources(app)
}

if err := v.syncPodGroup(app, minMember, totalResource); err != nil {
return err
}
return nil
return v.syncPodGroup(app, minMember, totalResource)
}

func getAppPodGroupName(app *rayv1.RayCluster) string {
Expand Down
10 changes: 5 additions & 5 deletions ray-operator/controllers/ray/common/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func DefaultHeadPodTemplate(ctx context.Context, instance rayv1.RayCluster, head
podTemplate.Labels = make(map[string]string)
}
podTemplate.Labels = labelPod(rayv1.HeadNode, instance.Name, utils.RayNodeHeadGroupLabelValue, instance.Spec.HeadGroupSpec.Template.ObjectMeta.Labels)
headSpec.RayStartParams = setMissingRayStartParams(ctx, headSpec.RayStartParams, rayv1.HeadNode, headPort, "", instance.Annotations)
headSpec.RayStartParams = setMissingRayStartParams(ctx, headSpec.RayStartParams, rayv1.HeadNode, headPort, "")

initTemplateAnnotations(instance, &podTemplate)

Expand Down Expand Up @@ -222,7 +222,7 @@ func DefaultWorkerPodTemplate(ctx context.Context, instance rayv1.RayCluster, wo
podTemplate.Labels = make(map[string]string)
}
podTemplate.Labels = labelPod(rayv1.WorkerNode, instance.Name, workerSpec.GroupName, workerSpec.Template.ObjectMeta.Labels)
workerSpec.RayStartParams = setMissingRayStartParams(ctx, workerSpec.RayStartParams, rayv1.WorkerNode, headPort, fqdnRayIP, instance.Annotations)
workerSpec.RayStartParams = setMissingRayStartParams(ctx, workerSpec.RayStartParams, rayv1.WorkerNode, headPort, fqdnRayIP)

initTemplateAnnotations(instance, &podTemplate)

Expand Down Expand Up @@ -666,7 +666,7 @@ func setContainerEnvVars(pod *corev1.Pod, rayNodeType rayv1.RayNodeType, rayStar
}
}

func setMissingRayStartParams(ctx context.Context, rayStartParams map[string]string, nodeType rayv1.RayNodeType, headPort string, fqdnRayIP string, annotations map[string]string) (completeStartParams map[string]string) {
func setMissingRayStartParams(ctx context.Context, rayStartParams map[string]string, nodeType rayv1.RayNodeType, headPort string, fqdnRayIP string) (completeStartParams map[string]string) {
log := ctrl.LoggerFrom(ctx)
// Note: The argument headPort is unused for nodeType == rayv1.HeadNode.
if nodeType == rayv1.WorkerNode {
Expand Down Expand Up @@ -777,7 +777,7 @@ func convertParamMap(rayStartParams map[string]string) (s string) {
func addEmptyDir(ctx context.Context, container *corev1.Container, pod *corev1.Pod, volumeName string, volumeMountPath string, storageMedium corev1.StorageMedium) {
log := ctrl.LoggerFrom(ctx)

if checkIfVolumeMounted(container, pod, volumeMountPath) {
if checkIfVolumeMounted(container, volumeMountPath) {
log.Info("volume already mounted", "volume", volumeName, "path", volumeMountPath)
return
}
Expand Down Expand Up @@ -822,7 +822,7 @@ func makeEmptyDirVolume(container *corev1.Container, volumeName string, storageM

// Checks if the container has a volumeMount with the given mount path and if
// the pod has a matching Volume.
func checkIfVolumeMounted(container *corev1.Container, pod *corev1.Pod, volumeMountPath string) bool {
func checkIfVolumeMounted(container *corev1.Container, volumeMountPath string) bool {
for _, mountedVol := range container.VolumeMounts {
if mountedVol.MountPath == volumeMountPath {
return true
Expand Down
32 changes: 16 additions & 16 deletions ray-operator/controllers/ray/common/pod_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -940,23 +940,23 @@ func TestSetMissingRayStartParamsAddress(t *testing.T) {

// Case 1: Head node with no address option set.
rayStartParams := map[string]string{}
rayStartParams = setMissingRayStartParams(ctx, rayStartParams, rayv1.HeadNode, headPort, "", nil)
rayStartParams = setMissingRayStartParams(ctx, rayStartParams, rayv1.HeadNode, headPort, "")
assert.NotContains(t, rayStartParams, "address", "Head node should not have an address option set by default.")

// Case 2: Head node with custom address option set.
rayStartParams = map[string]string{"address": customAddress}
rayStartParams = setMissingRayStartParams(ctx, rayStartParams, rayv1.HeadNode, headPort, "", nil)
rayStartParams = setMissingRayStartParams(ctx, rayStartParams, rayv1.HeadNode, headPort, "")
assert.Equal(t, customAddress, rayStartParams["address"], fmt.Sprintf("Expected `%v` but got `%v`", customAddress, rayStartParams["address"]))

// Case 3: Worker node with no address option set.
rayStartParams = map[string]string{}
rayStartParams = setMissingRayStartParams(ctx, rayStartParams, rayv1.WorkerNode, headPort, fqdnRayIP, nil)
rayStartParams = setMissingRayStartParams(ctx, rayStartParams, rayv1.WorkerNode, headPort, fqdnRayIP)
expectedAddress := fmt.Sprintf("%s:%s", fqdnRayIP, headPort)
assert.Equal(t, expectedAddress, rayStartParams["address"], fmt.Sprintf("Expected `%v` but got `%v`", expectedAddress, rayStartParams["address"]))

// Case 4: Worker node with custom address option set.
rayStartParams = map[string]string{"address": customAddress}
rayStartParams = setMissingRayStartParams(ctx, rayStartParams, rayv1.WorkerNode, headPort, fqdnRayIP, nil)
rayStartParams = setMissingRayStartParams(ctx, rayStartParams, rayv1.WorkerNode, headPort, fqdnRayIP)
assert.Equal(t, customAddress, rayStartParams["address"], fmt.Sprintf("Expected `%v` but got `%v`", customAddress, rayStartParams["address"]))
}

Expand All @@ -973,22 +973,22 @@ func TestSetMissingRayStartParamsMetricsExportPort(t *testing.T) {

// Case 1: Head node with no metrics-export-port option set.
rayStartParams := map[string]string{}
rayStartParams = setMissingRayStartParams(ctx, rayStartParams, rayv1.HeadNode, headPort, "", nil)
rayStartParams = setMissingRayStartParams(ctx, rayStartParams, rayv1.HeadNode, headPort, "")
assert.Equal(t, fmt.Sprint(utils.DefaultMetricsPort), rayStartParams["metrics-export-port"], fmt.Sprintf("Expected `%v` but got `%v`", fmt.Sprint(utils.DefaultMetricsPort), rayStartParams["metrics-export-port"]))

// Case 2: Head node with custom metrics-export-port option set.
rayStartParams = map[string]string{"metrics-export-port": fmt.Sprint(customMetricsPort)}
rayStartParams = setMissingRayStartParams(ctx, rayStartParams, rayv1.HeadNode, headPort, "", nil)
rayStartParams = setMissingRayStartParams(ctx, rayStartParams, rayv1.HeadNode, headPort, "")
assert.Equal(t, fmt.Sprint(customMetricsPort), rayStartParams["metrics-export-port"], fmt.Sprintf("Expected `%v` but got `%v`", fmt.Sprint(customMetricsPort), rayStartParams["metrics-export-port"]))

// Case 3: Worker node with no metrics-export-port option set.
rayStartParams = map[string]string{}
rayStartParams = setMissingRayStartParams(ctx, rayStartParams, rayv1.WorkerNode, headPort, fqdnRayIP, nil)
rayStartParams = setMissingRayStartParams(ctx, rayStartParams, rayv1.WorkerNode, headPort, fqdnRayIP)
assert.Equal(t, fmt.Sprint(utils.DefaultMetricsPort), rayStartParams["metrics-export-port"], fmt.Sprintf("Expected `%v` but got `%v`", fmt.Sprint(utils.DefaultMetricsPort), rayStartParams["metrics-export-port"]))

// Case 4: Worker node with custom metrics-export-port option set.
rayStartParams = map[string]string{"metrics-export-port": fmt.Sprint(customMetricsPort)}
rayStartParams = setMissingRayStartParams(ctx, rayStartParams, rayv1.WorkerNode, headPort, fqdnRayIP, nil)
rayStartParams = setMissingRayStartParams(ctx, rayStartParams, rayv1.WorkerNode, headPort, fqdnRayIP)
assert.Equal(t, fmt.Sprint(customMetricsPort), rayStartParams["metrics-export-port"], fmt.Sprintf("Expected `%v` but got `%v`", fmt.Sprint(customMetricsPort), rayStartParams["metrics-export-port"]))
}

Expand All @@ -1004,22 +1004,22 @@ func TestSetMissingRayStartParamsBlock(t *testing.T) {

// Case 1: Head node with no --block option set.
rayStartParams := map[string]string{}
rayStartParams = setMissingRayStartParams(ctx, rayStartParams, rayv1.HeadNode, headPort, "", nil)
rayStartParams = setMissingRayStartParams(ctx, rayStartParams, rayv1.HeadNode, headPort, "")
assert.Equal(t, "true", rayStartParams["block"], fmt.Sprintf("Expected `%v` but got `%v`", "true", rayStartParams["block"]))

// Case 2: Head node with --block option set to false.
rayStartParams = map[string]string{"block": "false"}
rayStartParams = setMissingRayStartParams(ctx, rayStartParams, rayv1.HeadNode, headPort, "", nil)
rayStartParams = setMissingRayStartParams(ctx, rayStartParams, rayv1.HeadNode, headPort, "")
assert.Equal(t, "true", rayStartParams["block"], fmt.Sprintf("Expected `%v` but got `%v`", "false", rayStartParams["block"]))

// Case 3: Worker node with no --block option set.
rayStartParams = map[string]string{}
rayStartParams = setMissingRayStartParams(ctx, rayStartParams, rayv1.WorkerNode, headPort, fqdnRayIP, nil)
rayStartParams = setMissingRayStartParams(ctx, rayStartParams, rayv1.WorkerNode, headPort, fqdnRayIP)
assert.Equal(t, "true", rayStartParams["block"], fmt.Sprintf("Expected `%v` but got `%v`", "true", rayStartParams["block"]))

// Case 4: Worker node with --block option set to false.
rayStartParams = map[string]string{"block": "false"}
rayStartParams = setMissingRayStartParams(ctx, rayStartParams, rayv1.WorkerNode, headPort, fqdnRayIP, nil)
rayStartParams = setMissingRayStartParams(ctx, rayStartParams, rayv1.WorkerNode, headPort, fqdnRayIP)
assert.Equal(t, "true", rayStartParams["block"], fmt.Sprintf("Expected `%v` but got `%v`", "false", rayStartParams["block"]))
}

Expand All @@ -1033,23 +1033,23 @@ func TestSetMissingRayStartParamsDashboardHost(t *testing.T) {

// Case 1: Head node with no dashboard-host option set.
rayStartParams := map[string]string{}
rayStartParams = setMissingRayStartParams(ctx, rayStartParams, rayv1.HeadNode, headPort, "", nil)
rayStartParams = setMissingRayStartParams(ctx, rayStartParams, rayv1.HeadNode, headPort, "")
assert.Equal(t, "0.0.0.0", rayStartParams["dashboard-host"], fmt.Sprintf("Expected `%v` but got `%v`", "0.0.0.0", rayStartParams["dashboard-host"]))

// Case 2: Head node with dashboard-host option set.
rayStartParams = map[string]string{"dashboard-host": "localhost"}
rayStartParams = setMissingRayStartParams(ctx, rayStartParams, rayv1.HeadNode, headPort, "", nil)
rayStartParams = setMissingRayStartParams(ctx, rayStartParams, rayv1.HeadNode, headPort, "")
assert.Equal(t, "localhost", rayStartParams["dashboard-host"], fmt.Sprintf("Expected `%v` but got `%v`", "localhost", rayStartParams["dashboard-host"]))

// Case 3: Worker node with no dashboard-host option set.
rayStartParams = map[string]string{}
rayStartParams = setMissingRayStartParams(ctx, rayStartParams, rayv1.WorkerNode, headPort, fqdnRayIP, nil)
rayStartParams = setMissingRayStartParams(ctx, rayStartParams, rayv1.WorkerNode, headPort, fqdnRayIP)
assert.NotContains(t, rayStartParams, "dashboard-host", "workers should not have an dashboard-host option set.")

// Case 4: Worker node with dashboard-host option set.
// To maximize user empowerment, this option can be enabled. However, it is important to note that the dashboard is not available on worker nodes.
rayStartParams = map[string]string{"dashboard-host": "localhost"}
rayStartParams = setMissingRayStartParams(ctx, rayStartParams, rayv1.WorkerNode, headPort, fqdnRayIP, nil)
rayStartParams = setMissingRayStartParams(ctx, rayStartParams, rayv1.WorkerNode, headPort, fqdnRayIP)
assert.Equal(t, "localhost", rayStartParams["dashboard-host"], fmt.Sprintf("Expected `%v` but got `%v`", "localhost", rayStartParams["dashboard-host"]))
}

Expand Down
Loading

0 comments on commit d149356

Please sign in to comment.