From 5bb9a281d9ab074adf5e088c0ad781bd6619e363 Mon Sep 17 00:00:00 2001 From: LeoLiao123 Date: Sun, 21 Apr 2024 14:29:50 +0800 Subject: [PATCH 1/2] change coding style in apiserver --- apiserver/pkg/model/converter.go | 6 +++--- apiserver/test/e2e/config_server_e2e_test.go | 12 ++++++------ apiserver/test/e2e/job_server_e2e_test.go | 12 ++++++------ 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/apiserver/pkg/model/converter.go b/apiserver/pkg/model/converter.go index 625ae82805..1cd5e88380 100755 --- a/apiserver/pkg/model/converter.go +++ b/apiserver/pkg/model/converter.go @@ -244,7 +244,7 @@ func PopulateHeadNodeSpec(spec rayv1api.HeadGroupSpec) *api.HeadGroupSpec { // Here we update environment only for a container named 'ray-head' if container, _, ok := util.GetContainerByName(spec.Template.Spec.Containers, "ray-head"); ok && len(container.Env) > 0 { - headNodeSpec.Environment = convert_env_variables(container.Env, true) + headNodeSpec.Environment = convertEnvVariables(container.Env, true) } if len(spec.Template.Spec.ServiceAccountName) > 1 { @@ -289,7 +289,7 @@ func PopulateWorkerNodeSpec(specs []rayv1api.WorkerGroupSpec) []*api.WorkerGroup // Here we update environment only for a container named 'ray-worker' if container, _, ok := util.GetContainerByName(spec.Template.Spec.Containers, "ray-worker"); ok && len(container.Env) > 0 { - workerNodeSpec.Environment = convert_env_variables(container.Env, false) + workerNodeSpec.Environment = convertEnvVariables(container.Env, false) } if len(spec.Template.Spec.ServiceAccountName) > 1 { @@ -306,7 +306,7 @@ func PopulateWorkerNodeSpec(specs []rayv1api.WorkerGroupSpec) []*api.WorkerGroup return workerNodeSpecs } -func convert_env_variables(cenv []corev1.EnvVar, header bool) *api.EnvironmentVariables { +func convertEnvVariables(cenv []corev1.EnvVar, header bool) *api.EnvironmentVariables { env := api.EnvironmentVariables{ Values: make(map[string]string), ValuesFrom: make(map[string]*api.EnvValueFrom), diff --git a/apiserver/test/e2e/config_server_e2e_test.go b/apiserver/test/e2e/config_server_e2e_test.go index 86dcd0e6c7..68fa97218a 100644 --- a/apiserver/test/e2e/config_server_e2e_test.go +++ b/apiserver/test/e2e/config_server_e2e_test.go @@ -207,14 +207,14 @@ func TestGetAllComputeTemplates(t *testing.T) { require.Nil(t, actualRpcStatus, "No RPC status expected") require.NotNil(t, response, "A response is expected") require.NotEmpty(t, response.ComputeTemplates, "A list of compute templates is required") - found_name := false + foundName := false for _, template := range response.ComputeTemplates { if tCtx.GetComputeTemplateName() == template.Name && tCtx.GetNamespaceName() == template.Namespace { - found_name = true + foundName = true break } } - require.Equal(t, found_name, true) + require.Equal(t, foundName, true) } // TestGetTemplatesInNamespace get all compute templates in namespace endpoint @@ -236,14 +236,14 @@ func TestGetTemplatesInNamespace(t *testing.T) { require.Nil(t, actualRpcStatus, "No RPC status expected") require.NotNil(t, response, "A response is expected") require.NotEmpty(t, response.ComputeTemplates, "A list of compute templates is required") - found_name := false + foundName := false for _, template := range response.ComputeTemplates { if tCtx.GetComputeTemplateName() == template.Name && tCtx.GetNamespaceName() == template.Namespace { - found_name = true + foundName = true break } } - require.Equal(t, found_name, true) + require.Equal(t, foundName, true) } // TestDeleteTemplate sequentially iterates over the delete compute template endpoint diff --git a/apiserver/test/e2e/job_server_e2e_test.go b/apiserver/test/e2e/job_server_e2e_test.go index 12e137edae..51dcc20060 100644 --- a/apiserver/test/e2e/job_server_e2e_test.go +++ b/apiserver/test/e2e/job_server_e2e_test.go @@ -303,14 +303,14 @@ func TestGetAllJobs(t *testing.T) { require.Nil(t, actualRpcStatus, "No RPC status expected") require.NotNil(t, response, "A response is expected") require.NotEmpty(t, response.Jobs, "A list of jobs is required") - found_name := false + foundName := false for _, job := range response.Jobs { if testJobRequest.Job.Name == job.Name && tCtx.GetNamespaceName() == job.Namespace { - found_name = true + foundName = true break } } - require.Equal(t, found_name, true) + require.Equal(t, foundName, true) } func TestGetJobsInNamespace(t *testing.T) { @@ -333,14 +333,14 @@ func TestGetJobsInNamespace(t *testing.T) { require.Nil(t, actualRpcStatus, "No RPC status expected") require.NotNil(t, response, "A response is expected") require.NotEmpty(t, response.Jobs, "A list of compute templates is required") - found_name := false + foundName := false for _, job := range response.Jobs { if testJobRequest.Job.Name == job.Name && tCtx.GetNamespaceName() == job.Namespace { - found_name = true + foundName = true break } } - require.Equal(t, found_name, true) + require.Equal(t, foundName, true) } func TestGetJob(t *testing.T) { From 13422d8a99b0c4fad81e3db951a1a9e0f7d57aea Mon Sep 17 00:00:00 2001 From: LeoLiao123 Date: Sun, 21 Apr 2024 15:32:30 +0800 Subject: [PATCH 2/2] change coding style in ray-operator --- .../controllers/ray/common/ingress.go | 6 +- .../controllers/ray/common/service.go | 92 +++++++++---------- .../controllers/ray/common/service_test.go | 30 +++--- 3 files changed, 64 insertions(+), 64 deletions(-) diff --git a/ray-operator/controllers/ray/common/ingress.go b/ray-operator/controllers/ray/common/ingress.go index 151f197a54..0d883e6a9d 100644 --- a/ray-operator/controllers/ray/common/ingress.go +++ b/ray-operator/controllers/ray/common/ingress.go @@ -27,15 +27,15 @@ func BuildIngressForHeadService(ctx context.Context, cluster rayv1.RayCluster) ( } // Copy other ingress configurations from cluster annotations to provide a generic way - // for user to customize their ingress settings. The `exclude_set` is used to avoid setting + // for user to customize their ingress settings. The `excludeSet` is used to avoid setting // both IngressClassAnnotationKey annotation which is deprecated and `Spec.IngressClassName` // at the same time. - exclude_set := map[string]struct{}{ + excludeSet := map[string]struct{}{ IngressClassAnnotationKey: {}, } annotation := map[string]string{} for key, value := range cluster.Annotations { - if _, ok := exclude_set[key]; !ok { + if _, ok := excludeSet[key]; !ok { annotation[key] = value } } diff --git a/ray-operator/controllers/ray/common/service.go b/ray-operator/controllers/ray/common/service.go index 7ffb8f58ee..c4a7f56dcc 100644 --- a/ray-operator/controllers/ray/common/service.go +++ b/ray-operator/controllers/ray/common/service.go @@ -33,40 +33,40 @@ func BuildServiceForHeadPod(ctx context.Context, cluster rayv1.RayCluster, label labels = make(map[string]string) } - default_labels := HeadServiceLabels(cluster) + defaultLabels := HeadServiceLabels(cluster) - // selector consists of *only* the keys in default_labels, updated with the values in labels if they exist + // selector consists of *only* the keys in defaultLabels, updated with the values in labels if they exist selector := make(map[string]string) - for k := range default_labels { + for k := range defaultLabels { if _, ok := labels[k]; ok { selector[k] = labels[k] } else { - selector[k] = default_labels[k] + selector[k] = defaultLabels[k] } } // Deep copy the selector to avoid modifying the original object - labels_for_service := make(map[string]string) + labelsForService := make(map[string]string) for k, v := range selector { - labels_for_service[k] = v + labelsForService[k] = v } if annotations == nil { annotations = make(map[string]string) } - default_name, err := utils.GenerateHeadServiceName(utils.RayClusterCRD, cluster.Spec, cluster.Name) + defaultName, err := utils.GenerateHeadServiceName(utils.RayClusterCRD, cluster.Spec, cluster.Name) if err != nil { return nil, err } - default_namespace := cluster.Namespace - default_type := cluster.Spec.HeadGroupSpec.ServiceType + defaultNamespace := cluster.Namespace + defaultType := cluster.Spec.HeadGroupSpec.ServiceType defaultAppProtocol := utils.DefaultServiceAppProtocol - // `ports_int` is a map of port names to port numbers, while `ports` is a list of ServicePort objects - ports_int := getServicePorts(cluster) + // `portsInt` is a map of port names to port numbers, while `ports` is a list of ServicePort objects + portsInt := getServicePorts(cluster) ports := []corev1.ServicePort{} - for name, port := range ports_int { + for name, port := range portsInt { svcPort := corev1.ServicePort{Name: name, Port: port, AppProtocol: &defaultAppProtocol} ports = append(ports, svcPort) } @@ -97,25 +97,25 @@ func BuildServiceForHeadPod(ctx context.Context, cluster rayv1.RayCluster, label // Append default ports. headService.Spec.Ports = append(headService.Spec.Ports, ports...) - setLabelsforUserProvidedService(headService, labels_for_service) - setNameforUserProvidedService(ctx, headService, default_name) - setNamespaceforUserProvidedService(ctx, headService, default_namespace) - setServiceTypeForUserProvidedService(ctx, headService, default_type) + setLabelsforUserProvidedService(headService, labelsForService) + setNameforUserProvidedService(ctx, headService, defaultName) + setNamespaceforUserProvidedService(ctx, headService, defaultNamespace) + setServiceTypeForUserProvidedService(ctx, headService, defaultType) return headService, nil } headService := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: default_name, - Namespace: default_namespace, - Labels: labels_for_service, + Name: defaultName, + Namespace: defaultNamespace, + Labels: labelsForService, Annotations: annotations, }, Spec: corev1.ServiceSpec{ Selector: selector, Ports: ports, - Type: default_type, + Type: defaultType, }, } @@ -191,17 +191,17 @@ func BuildServeService(ctx context.Context, rayService rayv1.RayService, rayClus selectorLabels[utils.RayClusterServingServiceLabelKey] = utils.EnableRayClusterServingServiceTrue } - default_name := utils.GenerateServeServiceName(name) - default_namespace := namespace - default_type := rayCluster.Spec.HeadGroupSpec.ServiceType + defaultName := utils.GenerateServeServiceName(name) + defaultNamespace := namespace + defaultType := rayCluster.Spec.HeadGroupSpec.ServiceType if isRayService { - default_type = rayService.Spec.RayClusterSpec.HeadGroupSpec.ServiceType + defaultType = rayService.Spec.RayClusterSpec.HeadGroupSpec.ServiceType } - // `ports_int` is a map of port names to port numbers, while `ports` is a list of ServicePort objects - ports_int := getServicePorts(rayCluster) + // `portsInt` is a map of port names to port numbers, while `ports` is a list of ServicePort objects + portsInt := getServicePorts(rayCluster) ports := []corev1.ServicePort{} - for name, port := range ports_int { + for name, port := range portsInt { if name == utils.ServingPortName { svcPort := corev1.ServicePort{Name: name, Port: port} ports = append(ports, svcPort) @@ -246,9 +246,9 @@ func BuildServeService(ctx context.Context, rayService rayv1.RayService, rayClus } setLabelsforUserProvidedService(serveService, labels) - setNameforUserProvidedService(ctx, serveService, default_name) - setNamespaceforUserProvidedService(ctx, serveService, default_namespace) - setServiceTypeForUserProvidedService(ctx, serveService, default_type) + setNameforUserProvidedService(ctx, serveService, defaultName) + setNamespaceforUserProvidedService(ctx, serveService, defaultNamespace) + setServiceTypeForUserProvidedService(ctx, serveService, defaultType) return serveService, nil } @@ -262,14 +262,14 @@ func BuildServeService(ctx context.Context, rayService rayv1.RayService, rayClus serveService := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: default_name, - Namespace: default_namespace, + Name: defaultName, + Namespace: defaultNamespace, Labels: labels, }, Spec: corev1.ServiceSpec{ Selector: selectorLabels, Ports: ports, - Type: default_type, + Type: defaultType, }, } @@ -305,45 +305,45 @@ func BuildHeadlessServiceForRayCluster(rayCluster rayv1.RayCluster) (*corev1.Ser return headlessService, nil } -func setServiceTypeForUserProvidedService(ctx context.Context, service *corev1.Service, default_type corev1.ServiceType) { +func setServiceTypeForUserProvidedService(ctx context.Context, service *corev1.Service, defaultType corev1.ServiceType) { log := ctrl.LoggerFrom(ctx) // If the user has not specified a service type, use the default service type if service.Spec.Type == "" { log.Info("Using default serviceType passed for the user provided service", - "default_type passed", default_type, + "default_type passed", defaultType, "service.ObjectMeta.Name", service.ObjectMeta.Name) - service.Spec.Type = default_type + service.Spec.Type = defaultType } else { log.Info("Overriding default serviceType with user provided serviceType", - "default_type passed", default_type, + "default_type passed", defaultType, "service.ObjectMeta.Name", service.ObjectMeta.Name, - "default_type passed", default_type, + "default_type passed", defaultType, "service.Spec.Type", service.Spec.Type) } } -func setNamespaceforUserProvidedService(ctx context.Context, service *corev1.Service, default_namespace string) { +func setNamespaceforUserProvidedService(ctx context.Context, service *corev1.Service, defaultNamespace string) { log := ctrl.LoggerFrom(ctx) // If the user has specified a namespace, ignore it and raise a warning - if service.ObjectMeta.Namespace != "" && service.ObjectMeta.Namespace != default_namespace { + if service.ObjectMeta.Namespace != "" && service.ObjectMeta.Namespace != defaultNamespace { log.Info("Ignoring namespace in user provided service", "provided_namespace", service.ObjectMeta.Namespace, "service_name", service.ObjectMeta.Name, - "default_namespace", default_namespace) + "default_namespace", defaultNamespace) } - service.ObjectMeta.Namespace = default_namespace + service.ObjectMeta.Namespace = defaultNamespace } -func setNameforUserProvidedService(ctx context.Context, service *corev1.Service, default_name string) { +func setNameforUserProvidedService(ctx context.Context, service *corev1.Service, defaultName string) { log := ctrl.LoggerFrom(ctx) // If the user has not specified a name, use the default name passed if service.ObjectMeta.Name == "" { - log.Info("Using default name for user provided service.", "default_name", default_name) - service.ObjectMeta.Name = default_name + log.Info("Using default name for user provided service.", "default_name", defaultName) + service.ObjectMeta.Name = defaultName } else { log.Info("Overriding default name for user provided service with name in service.ObjectMeta.Name.", - "default_name", default_name, + "default_name", defaultName, "provided_name", service.ObjectMeta.Name) } } diff --git a/ray-operator/controllers/ray/common/service_test.go b/ray-operator/controllers/ray/common/service_test.go index 148826158b..988af0a7ef 100644 --- a/ray-operator/controllers/ray/common/service_test.go +++ b/ray-operator/controllers/ray/common/service_test.go @@ -285,45 +285,45 @@ func TestUserSpecifiedHeadService(t *testing.T) { } // These labels originate from HeadGroupSpec.Template.ObjectMeta.Labels userTemplateClusterName := "userTemplateClusterName" - template_labels := map[string]string{utils.RayClusterLabelKey: userTemplateClusterName} - headService, err := BuildServiceForHeadPod(context.Background(), *testRayClusterWithHeadService, template_labels, testRayClusterWithHeadService.Spec.HeadServiceAnnotations) + templateLabels := map[string]string{utils.RayClusterLabelKey: userTemplateClusterName} + headService, err := BuildServiceForHeadPod(context.Background(), *testRayClusterWithHeadService, templateLabels, testRayClusterWithHeadService.Spec.HeadServiceAnnotations) if err != nil { t.Errorf("failed to build head service: %v", err) } // The selector field should only use the keys from the five default labels. The values should be updated with the values from the template labels. // The user-provided HeadService labels should be ignored for the purposes of the selector field. The user-provided Selector field should be ignored. - default_labels := HeadServiceLabels(*testRayClusterWithHeadService) + defaultLabels := HeadServiceLabels(*testRayClusterWithHeadService) // Make sure this test isn't spuriously passing. Check that RayClusterLabelKey is in the default labels. - if _, ok := default_labels[utils.RayClusterLabelKey]; !ok { + if _, ok := defaultLabels[utils.RayClusterLabelKey]; !ok { t.Errorf("utils.RayClusterLabelKey=%s should be in the default labels", utils.RayClusterLabelKey) } for k, v := range headService.Spec.Selector { // If k is not in the default labels, then the selector field should not contain it. - if _, ok := default_labels[k]; !ok { + if _, ok := defaultLabels[k]; !ok { t.Errorf("Selector field should not contain key=%s", k) } // If k is in the template labels, then the selector field should contain it with the value from the template labels. // Otherwise, it should contain the value from the default labels. - if _, ok := template_labels[k]; ok { - if v != template_labels[k] { - t.Errorf("Selector field should contain key=%s with value=%s, actual value=%s", k, template_labels[k], v) + if _, ok := templateLabels[k]; ok { + if v != templateLabels[k] { + t.Errorf("Selector field should contain key=%s with value=%s, actual value=%s", k, templateLabels[k], v) } } else { - if v != default_labels[k] { - t.Errorf("Selector field should contain key=%s with value=%s, actual value=%s", k, default_labels[k], v) + if v != defaultLabels[k] { + t.Errorf("Selector field should contain key=%s with value=%s, actual value=%s", k, defaultLabels[k], v) } } } // The selector field should have every key from the default labels. - for k := range default_labels { + for k := range defaultLabels { if _, ok := headService.Spec.Selector[k]; !ok { t.Errorf("Selector field should contain key=%s", k) } } // Print default labels for debugging - for k, v := range default_labels { + for k, v := range defaultLabels { fmt.Printf("default label: key=%s, value=%s\n", k, v) } @@ -593,7 +593,7 @@ func validateServiceTypeForUserSpecifiedService(svc *corev1.Service, userType co } } -func validateNameAndNamespaceForUserSpecifiedService(svc *corev1.Service, default_namespace string, userName string, t *testing.T) { +func validateNameAndNamespaceForUserSpecifiedService(svc *corev1.Service, defaultNamespace string, userName string, t *testing.T) { // Test name and namespace are generated if not specified if svc.ObjectMeta.Name == "" { t.Errorf("Generated service name is empty") @@ -602,8 +602,8 @@ func validateNameAndNamespaceForUserSpecifiedService(svc *corev1.Service, defaul t.Errorf("Generated service namespace is empty") } // The user-provided namespace should be ignored, but the name should be respected - if svc.ObjectMeta.Namespace != default_namespace { - t.Errorf("User-provided namespace should be ignored: expected namespace=%s, actual namespace=%s", default_namespace, svc.ObjectMeta.Namespace) + if svc.ObjectMeta.Namespace != defaultNamespace { + t.Errorf("User-provided namespace should be ignored: expected namespace=%s, actual namespace=%s", defaultNamespace, svc.ObjectMeta.Namespace) } if svc.ObjectMeta.Name != userName { t.Errorf("User-provided name should be respected: expected name=%s, actual name=%s", userName, svc.ObjectMeta.Name)