Skip to content

WIP: change ep service bind #125

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
Airren wants to merge 2 commits into
base: main
Choose a base branch
from
Open

WIP: change ep service bind #125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e5da154
feat: change ep service bind
May 12, 2025
cd3a649
feat: change ep service bind
May 13, 2025
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
18 changes: 9 additions & 9 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ else
GIT_VERSION=$(shell git describe --abbrev=0 --tags --always)
endif

IMAGE_TAG = ${GIT_VERSION}
IMAGE_TAG = svctest
REGISTRY ?= ghcr.io
REGISTRY_NAMESPACE ?= fleetboard-io

Expand Down Expand Up @@ -81,10 +81,10 @@ image-crossdns:
docker push ${REGISTRY}/${REGISTRY_NAMESPACE}/crossdns:latest

image-cnf:
docker buildx build --platform linux/amd64,linux/arm64 $(DOCKERARGS) -f ./build/cnf.Dockerfile ./ -t ${REGISTRY}/${REGISTRY_NAMESPACE}/cnf:${IMAGE_TAG}
docker push ${REGISTRY}/${REGISTRY_NAMESPACE}/cnf:${IMAGE_TAG}
docker tag ${REGISTRY}/${REGISTRY_NAMESPACE}/cnf:${IMAGE_TAG} ${REGISTRY}/${REGISTRY_NAMESPACE}/cnf:latest
docker push ${REGISTRY}/${REGISTRY_NAMESPACE}/cnf:latest
docker buildx build --platform linux/amd64,linux/arm64 $(DOCKERARGS) -f ./build/cnf.Dockerfile ./ -t ${REGISTRY}/${REGISTRY_NAMESPACE}/cnf:${IMAGE_TAG} --push
# docker push ${REGISTRY}/${REGISTRY_NAMESPACE}/cnf:${IMAGE_TAG}
# docker tag ${REGISTRY}/${REGISTRY_NAMESPACE}/cnf:${IMAGE_TAG} ${REGISTRY}/${REGISTRY_NAMESPACE}/cnf:latest
# docker push ${REGISTRY}/${REGISTRY_NAMESPACE}/cnf:latest

image-proxy:
docker buildx build --platform linux/amd64,linux/arm64 $(DOCKERARGS) -f ./build/proxy.Dockerfile ./ -t ${REGISTRY}/${REGISTRY_NAMESPACE}/proxy:${IMAGE_TAG}
Expand All @@ -93,10 +93,10 @@ image-proxy:
docker push ${REGISTRY}/${REGISTRY_NAMESPACE}/proxy:latest

image-ep-controller:
docker buildx build --platform linux/amd64,linux/arm64 $(DOCKERARGS) -f ./build/ep-controller.Dockerfile ./ -t ${REGISTRY}/${REGISTRY_NAMESPACE}/controller:${IMAGE_TAG}
docker push ${REGISTRY}/${REGISTRY_NAMESPACE}/controller:${IMAGE_TAG}
docker tag ${REGISTRY}/${REGISTRY_NAMESPACE}/controller:${IMAGE_TAG} ${REGISTRY}/${REGISTRY_NAMESPACE}/controller:latest
docker push ${REGISTRY}/${REGISTRY_NAMESPACE}/controller:latest
docker buildx build --platform linux/amd64,linux/arm64 $(DOCKERARGS) -f ./build/ep-controller.Dockerfile ./ -t ${REGISTRY}/${REGISTRY_NAMESPACE}/controller:${IMAGE_TAG} --push
#docker push ${REGISTRY}/${REGISTRY_NAMESPACE}/controller:${IMAGE_TAG}
#docker tag ${REGISTRY}/${REGISTRY_NAMESPACE}/controller:${IMAGE_TAG} ${REGISTRY}/${REGISTRY_NAMESPACE}/controller:latest
#docker push ${REGISTRY}/${REGISTRY_NAMESPACE}/controller:latest


images-push: image-crossdns image-cnf image-proxy image-ep-controller
Expand Down
3 changes: 3 additions & 0 deletions build/cnf.Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ FROM golang:1.21-alpine AS builder
WORKDIR /workspace
RUN apk update && apk add --no-cache make git
COPY ../go.mod ../go.sum ./
# ENV GOPROXY='https://goproxy.io,direct'
ENV GOPROXY=https://goproxy.cn,direct

Comment on lines +6 to +8
Copy link

Copilot AI Jun 9, 2025

Choose a reason for hiding this comment

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

[nitpick] Remove or consolidate commented-out proxy lines to keep the Dockerfile clean and avoid confusion over which proxy is intended.

Suggested change
# ENV GOPROXY='https://goproxy.io,direct'
ENV GOPROXY=https://goproxy.cn,direct
ENV GOPROXY=https://goproxy.cn,direct

Copilot uses AI. Check for mistakes.
RUN go mod download
COPY .. .
RUN make cnf
Expand Down
1 change: 1 addition & 0 deletions build/ep-controller.Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ FROM golang:1.21-alpine AS builder
WORKDIR /workspace
RUN apk add make
COPY ../go.mod ../go.sum ./
ENV GOPROXY=https://goproxy.cn,direct
RUN go mod download
COPY .. .
RUN make ep-controller
Expand Down
Binary file added cmd/ep-controller/ep-controller
View file
Open in desktop
Binary file not shown.
6 changes: 3 additions & 3 deletions pkg/controller/endpointslice/endpointslice.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ func (c *Controller) syncService(logger klog.Logger, key string) error {
}

esLabelSelector := labels.Set(map[string]string{
discovery.LabelServiceName: service.Name,
discovery.LabelServiceName: endpointsliceutil.GetServiceLabelFromSvcName(service.Name),
discovery.LabelManagedBy: c.reconciler.GetControllerName(),
}).AsSelectorPreValidated()
endpointSlices, err := c.endpointSliceLister.EndpointSlices(service.Namespace).List(esLabelSelector)
Expand Down Expand Up @@ -461,8 +461,8 @@ func (c *Controller) onEndpointSliceUpdate(logger klog.Logger, prevObj, obj inte
// EndpointSlice generation does not change when labels change. Although the
// controller will never change LabelServiceName, users might. This check
// ensures that we handle changes to this label.
svcName := endpointSlice.Labels[discovery.LabelServiceName]
prevSvcName := prevEndpointSlice.Labels[discovery.LabelServiceName]
svcName, _ := endpointsliceutil.GetServiceNameFromEpLabel(endpointSlice.Labels[discovery.LabelServiceName])
prevSvcName, _ := endpointsliceutil.GetServiceNameFromEpLabel(prevEndpointSlice.Labels[discovery.LabelServiceName])
Comment on lines +464 to +465
Copy link

Copilot AI Jun 9, 2025

Choose a reason for hiding this comment

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

Ignoring the error returned by GetServiceNameFromEpLabel may mask parsing failures. Consider handling the error or logging it.

Suggested change
svcName, _ := endpointsliceutil.GetServiceNameFromEpLabel(endpointSlice.Labels[discovery.LabelServiceName])
prevSvcName, _ := endpointsliceutil.GetServiceNameFromEpLabel(prevEndpointSlice.Labels[discovery.LabelServiceName])
svcName, err := endpointsliceutil.GetServiceNameFromEpLabel(endpointSlice.Labels[discovery.LabelServiceName])
if err != nil {
logger.Error(err, "failed to parse service name from EndpointSlice label", "endpointslice", klog.KObj(endpointSlice))
return
}
prevSvcName, err := endpointsliceutil.GetServiceNameFromEpLabel(prevEndpointSlice.Labels[discovery.LabelServiceName])
if err != nil {
logger.Error(err, "failed to parse service name from previous EndpointSlice label", "endpointslice", klog.KObj(prevEndpointSlice))
return
}

Copilot uses AI. Check for mistakes.
if svcName != prevSvcName {
logger.Info("label changed", "label", discovery.LabelServiceName, "oldService",
prevSvcName, "newService", svcName, "endpointslice", klog.KObj(endpointSlice))
Expand Down
18 changes: 17 additions & 1 deletion pkg/controller/endpointslice/util/endpointslice_tracker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ limitations under the License.
package util

import (
"fmt"
"strings"
"sync"

v1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -187,6 +189,20 @@ func (est *EndpointSliceTracker) GenerationsForSliceUnsafe(endpointSlice *discov
// getServiceNN returns a namespaced name for the Service corresponding to the
// provided EndpointSlice.
func getServiceNN(endpointSlice *discovery.EndpointSlice) types.NamespacedName {
serviceName := endpointSlice.Labels[discovery.LabelServiceName]
serviceName, _ := GetServiceNameFromEpLabel(endpointSlice.Labels[discovery.LabelServiceName])
Copy link

Copilot AI Jun 9, 2025

Choose a reason for hiding this comment

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

The error from GetServiceNameFromEpLabel is ignored here, which can lead to empty serviceName. Handle or log the error to avoid silent failures.

Suggested change
serviceName, _ := GetServiceNameFromEpLabel(endpointSlice.Labels[discovery.LabelServiceName])
serviceName, err := GetServiceNameFromEpLabel(endpointSlice.Labels[discovery.LabelServiceName])
if err != nil {
fmt.Printf("Error extracting service name from label: %v\n", err)
return types.NamespacedName{Name: "", Namespace: endpointSlice.Namespace}
}

Copilot uses AI. Check for mistakes.
return types.NamespacedName{Name: serviceName, Namespace: endpointSlice.Namespace}
}

func GetServiceLabelFromSvcName(serviceName string) string {
return fmt.Sprintf("%s-----%s", "nauti-service", serviceName)
}
func GetServiceNameFromEpLabel(label string) (string, error) {
parts := strings.Split(label, "-----")
Comment on lines +197 to +200
Copy link

Copilot AI Jun 9, 2025

Choose a reason for hiding this comment

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

[nitpick] The delimiter "-----" is arbitrary and could conflict with service names. Consider defining it as a constant or choosing a safer separator.

Suggested change
return fmt.Sprintf("%s-----%s", "nauti-service", serviceName)
}
func GetServiceNameFromEpLabel(label string) (string, error) {
parts := strings.Split(label, "-----")
return fmt.Sprintf("%s%s%s", "nauti-service", serviceLabelDelimiter, serviceName)
}
func GetServiceNameFromEpLabel(label string) (string, error) {
parts := strings.Split(label, serviceLabelDelimiter)

Copilot uses AI. Check for mistakes.
if len(parts) != 2 {
return "", fmt.Errorf("invalid label format")
Copy link

Copilot AI Jun 9, 2025

Choose a reason for hiding this comment

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

Make this error more descriptive by including the invalid label value to aid diagnostics, e.g., fmt.Errorf("invalid label format: %q", label).

Suggested change
return "", fmt.Errorf("invalid label format")
return "", fmt.Errorf("invalid label format: %q", label)

Copilot uses AI. Check for mistakes.
}
if parts[0] != "nauti-service" {
Comment on lines +197 to +204
Copy link

Copilot AI Jun 9, 2025

Choose a reason for hiding this comment

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

[nitpick] Avoid using the literal prefix "nauti-service" here; extract it (and the delimiter) into package-level constants to reduce magic strings and improve consistency.

Suggested change
return fmt.Sprintf("%s-----%s", "nauti-service", serviceName)
}
func GetServiceNameFromEpLabel(label string) (string, error) {
parts := strings.Split(label, "-----")
if len(parts) != 2 {
return "", fmt.Errorf("invalid label format")
}
if parts[0] != "nauti-service" {
return fmt.Sprintf("%s%s%s", serviceLabelPrefix, serviceLabelDelimiter, serviceName)
}
func GetServiceNameFromEpLabel(label string) (string, error) {
parts := strings.Split(label, serviceLabelDelimiter)
if len(parts) != 2 {
return "", fmt.Errorf("invalid label format")
}
if parts[0] != serviceLabelPrefix {

Copilot uses AI. Check for mistakes.
return "", fmt.Errorf("invalid label prefix")
}
return parts[1], nil
}
8 changes: 5 additions & 3 deletions pkg/controller/endpointslice/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ import (
"k8s.io/apimachinery/pkg/util/sets"
endpointutil "k8s.io/endpointslice/util"
utilnet "k8s.io/utils/net"

endpointsliceutil "github.com/fleetboard-io/fleetboard/pkg/controller/endpointslice/util"
)

func GetDedicatedCNIIP(pod *v1.Pod) (ip net.IP, err error) {
Expand Down Expand Up @@ -210,8 +212,8 @@ func ServiceControllerKey(endpointSlice *discovery.EndpointSlice) (string, error
if endpointSlice == nil {
return "", fmt.Errorf("nil EndpointSlice passed to ServiceControllerKey()")
}
serviceName, ok := endpointSlice.Labels[discovery.LabelServiceName]
if !ok || serviceName == "" {
serviceName, err := endpointsliceutil.GetServiceNameFromEpLabel(endpointSlice.Labels[discovery.LabelServiceName])
if err != nil || serviceName == "" {
return "", fmt.Errorf("EndpointSlice missing %s label", discovery.LabelServiceName)
}
return fmt.Sprintf("%s/%s", endpointSlice.Namespace, serviceName), nil
Expand Down Expand Up @@ -256,7 +258,7 @@ func setEndpointSliceLabels(logger klog.Logger, epSlice *discovery.EndpointSlice
// add or remove headless label depending on the service Type

// override endpoint slices reserved labels
svcLabels[discovery.LabelServiceName] = service.Name
svcLabels[discovery.LabelServiceName] = endpointsliceutil.GetServiceLabelFromSvcName(service.Name)
svcLabels[discovery.LabelManagedBy] = controllerName
svcLabels[v1.IsHeadlessService] = "true"

Expand Down
Loading