Skip to content

Commit

Permalink
Ensure container ports without names are also included in the head no…
Browse files Browse the repository at this point in the history
…de service (#891)

Container ports without names will not be ignored.
  • Loading branch information
Yicheng-Lu-llll authored Feb 14, 2023
1 parent 2fa40fb commit 6490749
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 2 deletions.
5 changes: 5 additions & 0 deletions ray-operator/controllers/ray/common/service.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package common

import (
"fmt"

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

Expand Down Expand Up @@ -181,6 +183,9 @@ func getPortsFromCluster(cluster rayiov1alpha1.RayCluster) (map[string]int32, er
index := utils.FindRayContainerIndex(cluster.Spec.HeadGroupSpec.Template.Spec)
cPorts := cluster.Spec.HeadGroupSpec.Template.Spec.Containers[index].Ports
for _, port := range cPorts {
if port.Name == "" {
port.Name = fmt.Sprint(port.ContainerPort) + "-port"
}
svcPorts[port.Name] = port.ContainerPort
}

Expand Down
43 changes: 41 additions & 2 deletions ray-operator/controllers/ray/common/service_test.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package common

import (
"fmt"
"reflect"
"testing"

rayiov1alpha1 "github.com/ray-project/kuberay/ray-operator/apis/ray/v1alpha1"
"github.com/ray-project/kuberay/ray-operator/controllers/ray/utils"

"github.com/stretchr/testify/assert"

Expand Down Expand Up @@ -40,8 +42,17 @@ var instanceWithWrongSvc = &rayiov1alpha1.RayCluster{
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{
Name: "ray-head",
Image: "rayproject/autoscaler",
Name: "ray-head",
Image: "rayproject/autoscaler",
Ports: []corev1.ContainerPort{
{
ContainerPort: 6379,
Name: "gcs",
},
{
ContainerPort: 8265,
},
},
Command: []string{"python"},
Args: []string{"/opt/code.py"},
Env: []corev1.EnvVar{
Expand Down Expand Up @@ -126,3 +137,31 @@ func TestBuildServiceForHeadPodWithAnnotations(t *testing.T) {
t.Fatalf("Expected `%v` but got `%v`", annotations, svc.ObjectMeta.Annotations)
}
}

func TestGetPortsFromCluster(t *testing.T) {
svcPorts, err := getPortsFromCluster(*instanceWithWrongSvc)
assert.Nil(t, err)

// getPortsFromCluster creates service ports based on the container ports.
// It will assign a generated service port name if the container port name
// is not defined. To compare created service ports with container ports,
// all generated service port names need to be reverted to empty strings.
svcNames := map[int32]string{}
for name, port := range svcPorts {
if name == (fmt.Sprint(port) + "-port") {
name = ""
}
svcNames[port] = name
}

index := utils.FindRayContainerIndex(instanceWithWrongSvc.Spec.HeadGroupSpec.Template.Spec)
cPorts := instanceWithWrongSvc.Spec.HeadGroupSpec.Template.Spec.Containers[index].Ports

for _, cPort := range cPorts {
expectedResult := cPort.Name
actualResult := svcNames[cPort.ContainerPort]
if !reflect.DeepEqual(expectedResult, actualResult) {
t.Fatalf("Expected `%v` but got `%v`", expectedResult, actualResult)
}
}
}

0 comments on commit 6490749

Please sign in to comment.