Skip to content

Commit

Permalink
slice mirroring controller mirror annotations
Browse files Browse the repository at this point in the history 
Add support to the endpoint slice mirroring controller to mirror
annotations, in addition to labels, but don´t mirror endpoint
triggertime annotation.

Also, fix a bug in the endpointslice mirroring controller, that
wasn't updating the mirrored slice with the new labels, in case
that only the endpoint labels were modified.
  • Loading branch information
@aojea
Antonio Ojea authored and aojea committed Mar 22, 2021
1 parent 6d41a99 commit a53e27f
Show file tree
Hide file tree
Showing 6 changed files with 463 additions and 25 deletions.
10 changes: 8 additions & 2 deletions pkg/controller/endpointslicemirroring/reconciler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (

corev1 "k8s.io/api/core/v1"
discovery "k8s.io/api/discovery/v1beta1"
apiequality "k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
Expand Down Expand Up @@ -196,9 +197,14 @@ func (r *reconciler) reconcileByPortMapping(
// if >0 existing slices, mark all but 1 for deletion.
slices.toDelete = existingSlices[1:]

// Return early if first slice matches desired endpoints.
// generated slices must mirror all endpoints annotations but EndpointsLastChangeTriggerTime
compareAnnotations := cloneAndRemoveKeys(endpoints.Annotations, corev1.EndpointsLastChangeTriggerTime)
compareLabels := cloneAndRemoveKeys(existingSlices[0].Labels, discovery.LabelManagedBy, discovery.LabelServiceName)
// Return early if first slice matches desired endpoints, labels and annotations
totals = totalChanges(existingSlices[0], desiredSet)
if totals.added == 0 && totals.updated == 0 && totals.removed == 0 {
if totals.added == 0 && totals.updated == 0 && totals.removed == 0 &&
apiequality.Semantic.DeepEqual(endpoints.Labels, compareLabels) &&
apiequality.Semantic.DeepEqual(compareAnnotations, existingSlices[0].Annotations) {
return slices, totals
}
}
Expand Down
99 changes: 98 additions & 1 deletion pkg/controller/endpointslicemirroring/reconciler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ func TestReconcile(t *testing.T) {
testCases := []struct {
testName string
subsets []corev1.EndpointSubset
epLabels map[string]string
endpointsDeletionPending bool
maxEndpointsPerSubset int32
existingEndpointSlices []*discovery.EndpointSlice
Expand Down Expand Up @@ -105,6 +106,102 @@ func TestReconcile(t *testing.T) {
existingEndpointSlices: []*discovery.EndpointSlice{},
expectedNumSlices: 0,
expectedClientActions: 0,
}, {
testName: "Endpoints with 1 subset, port, and address and existing slice with same fields",
subsets: []corev1.EndpointSubset{{
Ports: []corev1.EndpointPort{{
Name: "http",
Port: 80,
Protocol: corev1.ProtocolTCP,
}},
Addresses: []corev1.EndpointAddress{{
IP: "10.0.0.1",
Hostname: "pod-1",
}},
}},
existingEndpointSlices: []*discovery.EndpointSlice{{
ObjectMeta: metav1.ObjectMeta{
Name: "test-ep-1",
},
AddressType: discovery.AddressTypeIPv4,
Ports: []discovery.EndpointPort{{
Name: utilpointer.StringPtr("http"),
Port: utilpointer.Int32Ptr(80),
Protocol: &protoTCP,
}},
Endpoints: []discovery.Endpoint{{
Addresses: []string{"10.0.0.1"},
Hostname: utilpointer.StringPtr("pod-1"),
Conditions: discovery.EndpointConditions{Ready: utilpointer.BoolPtr(true)},
}},
}},
expectedNumSlices: 1,
expectedClientActions: 0,
}, {
testName: "Endpoints with 1 subset, port, and address and existing slice with an additional annotation",
subsets: []corev1.EndpointSubset{{
Ports: []corev1.EndpointPort{{
Name: "http",
Port: 80,
Protocol: corev1.ProtocolTCP,
}},
Addresses: []corev1.EndpointAddress{{
IP: "10.0.0.1",
Hostname: "pod-1",
}},
}},
existingEndpointSlices: []*discovery.EndpointSlice{{
ObjectMeta: metav1.ObjectMeta{
Name: "test-ep-1",
Annotations: map[string]string{"foo": "bar"},
},
AddressType: discovery.AddressTypeIPv4,
Ports: []discovery.EndpointPort{{
Name: utilpointer.StringPtr("http"),
Port: utilpointer.Int32Ptr(80),
Protocol: &protoTCP,
}},
Endpoints: []discovery.Endpoint{{
Addresses: []string{"10.0.0.1"},
Hostname: utilpointer.StringPtr("pod-1"),
Conditions: discovery.EndpointConditions{Ready: utilpointer.BoolPtr(true)},
}},
}},
expectedNumSlices: 1,
expectedClientActions: 1,
}, {
testName: "Endpoints with 1 subset, port, label and address and existing slice with same fields but the label",
subsets: []corev1.EndpointSubset{{
Ports: []corev1.EndpointPort{{
Name: "http",
Port: 80,
Protocol: corev1.ProtocolTCP,
}},
Addresses: []corev1.EndpointAddress{{
IP: "10.0.0.1",
Hostname: "pod-1",
}},
}},
epLabels: map[string]string{"foo": "bar"},
existingEndpointSlices: []*discovery.EndpointSlice{{
ObjectMeta: metav1.ObjectMeta{
Name: "test-ep-1",
Annotations: map[string]string{"foo": "bar"},
},
AddressType: discovery.AddressTypeIPv4,
Ports: []discovery.EndpointPort{{
Name: utilpointer.StringPtr("http"),
Port: utilpointer.Int32Ptr(80),
Protocol: &protoTCP,
}},
Endpoints: []discovery.Endpoint{{
Addresses: []string{"10.0.0.1"},
Hostname: utilpointer.StringPtr("pod-1"),
Conditions: discovery.EndpointConditions{Ready: utilpointer.BoolPtr(true)},
}},
}},
expectedNumSlices: 1,
expectedClientActions: 1,
}, {
testName: "Endpoints with 1 subset, 2 ports, and 2 addresses",
subsets: []corev1.EndpointSubset{{
Expand Down Expand Up @@ -641,7 +738,7 @@ func TestReconcile(t *testing.T) {
setupMetrics()
namespace := "test"
endpoints := corev1.Endpoints{
ObjectMeta: metav1.ObjectMeta{Name: "test-ep", Namespace: namespace},
ObjectMeta: metav1.ObjectMeta{Name: "test-ep", Namespace: namespace, Labels: tc.epLabels},
Subsets: tc.subsets,
}

Expand Down
30 changes: 30 additions & 0 deletions pkg/controller/endpointslicemirroring/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ func newEndpointSlice(endpoints *corev1.Endpoints, ports []discovery.EndpointPor
epSlice := &discovery.EndpointSlice{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{},
Annotations: map[string]string{},
OwnerReferences: []metav1.OwnerReference{*ownerRef},
Namespace: endpoints.Namespace,
},
Expand All @@ -99,13 +100,23 @@ func newEndpointSlice(endpoints *corev1.Endpoints, ports []discovery.EndpointPor
Endpoints: []discovery.Endpoint{},
}

// clone all labels
for label, val := range endpoints.Labels {
epSlice.Labels[label] = val
}

// overwrite specific labels
epSlice.Labels[discovery.LabelServiceName] = endpoints.Name
epSlice.Labels[discovery.LabelManagedBy] = controllerName

// clone all annotations but EndpointsLastChangeTriggerTime
for annotation, val := range endpoints.Annotations {
if annotation == corev1.EndpointsLastChangeTriggerTime {
continue
}
epSlice.Annotations[annotation] = val
}

if sliceName == "" {
epSlice.GenerateName = getEndpointSlicePrefix(endpoints.Name)
} else {
Expand Down Expand Up @@ -278,3 +289,22 @@ func hasLeaderElection(annotations map[string]string) bool {
_, ok := annotations[resourcelock.LeaderElectionRecordAnnotationKey]
return ok
}

// cloneAndRemoveKeys is a copy of CloneAndRemoveLabels
// it is used here for annotations and labels
func cloneAndRemoveKeys(a map[string]string, keys ...string) map[string]string {
if len(keys) == 0 {
// Don't need to remove a key.
return a
}
// Clone.
newMap := map[string]string{}
for k, v := range a {
newMap[k] = v
}
// remove keys
for _, key := range keys {
delete(newMap, key)
}
return newMap
}
156 changes: 134 additions & 22 deletions pkg/controller/endpointslicemirroring/utils_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"testing"

"github.com/stretchr/testify/assert"
corev1 "k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
discovery "k8s.io/api/discovery/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -41,42 +42,153 @@ func TestNewEndpointSlice(t *testing.T) {

ports := []discovery.EndpointPort{{Name: &portName, Protocol: &protocol}}
addrType := discovery.AddressTypeIPv4
gvk := schema.GroupVersionKind{Version: "v1", Kind: "Endpoints"}

endpoints := v1.Endpoints{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "test",
Labels: map[string]string{"foo": "bar"},
},
Subsets: []v1.EndpointSubset{{
Ports: []v1.EndpointPort{{Port: 80}},
}},
}

gvk := schema.GroupVersionKind{Version: "v1", Kind: "Endpoints"}
ownerRef := metav1.NewControllerRef(&endpoints, gvk)

expectedSlice := discovery.EndpointSlice{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
"foo": "bar",
discovery.LabelServiceName: endpoints.Name,
discovery.LabelManagedBy: controllerName,
},
GenerateName: fmt.Sprintf("%s-", endpoints.Name),
OwnerReferences: []metav1.OwnerReference{*ownerRef},
Namespace: endpoints.Namespace,
testCases := []struct {
name string
tweakEndpoint func(ep *corev1.Endpoints)
expectedSlice discovery.EndpointSlice
}{
{
name: "create slice from endpoints",
tweakEndpoint: func(ep *corev1.Endpoints) {
},
expectedSlice: discovery.EndpointSlice{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
discovery.LabelServiceName: endpoints.Name,
discovery.LabelManagedBy: controllerName,
},
Annotations: map[string]string{},
GenerateName: fmt.Sprintf("%s-", endpoints.Name),
Namespace: endpoints.Namespace,
OwnerReferences: []metav1.OwnerReference{*ownerRef},
},
Ports: ports,
AddressType: addrType,
Endpoints: []discovery.Endpoint{},
},
},
{
name: "create slice from endpoints with annotations",
tweakEndpoint: func(ep *corev1.Endpoints) {
annotations := map[string]string{"foo": "bar"}
ep.Annotations = annotations
},
expectedSlice: discovery.EndpointSlice{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
discovery.LabelServiceName: endpoints.Name,
discovery.LabelManagedBy: controllerName,
},
Annotations: map[string]string{"foo": "bar"},
GenerateName: fmt.Sprintf("%s-", endpoints.Name),
Namespace: endpoints.Namespace,
OwnerReferences: []metav1.OwnerReference{*ownerRef},
},
Ports: ports,
AddressType: addrType,
Endpoints: []discovery.Endpoint{},
},
},
{
name: "create slice from endpoints with labels",
tweakEndpoint: func(ep *corev1.Endpoints) {
labels := map[string]string{"foo": "bar"}
ep.Labels = labels
},
expectedSlice: discovery.EndpointSlice{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
"foo": "bar",
discovery.LabelServiceName: endpoints.Name,
discovery.LabelManagedBy: controllerName,
},
Annotations: map[string]string{},
GenerateName: fmt.Sprintf("%s-", endpoints.Name),
Namespace: endpoints.Namespace,
OwnerReferences: []metav1.OwnerReference{*ownerRef},
},
Ports: ports,
AddressType: addrType,
Endpoints: []discovery.Endpoint{},
},
},
{
name: "create slice from endpoints with labels and annotations",
tweakEndpoint: func(ep *corev1.Endpoints) {
labels := map[string]string{"foo": "bar"}
ep.Labels = labels
annotations := map[string]string{"foo2": "bar2"}
ep.Annotations = annotations
},
expectedSlice: discovery.EndpointSlice{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
"foo": "bar",
discovery.LabelServiceName: endpoints.Name,
discovery.LabelManagedBy: controllerName,
},
Annotations: map[string]string{"foo2": "bar2"},
GenerateName: fmt.Sprintf("%s-", endpoints.Name),
Namespace: endpoints.Namespace,
OwnerReferences: []metav1.OwnerReference{*ownerRef},
},
Ports: ports,
AddressType: addrType,
Endpoints: []discovery.Endpoint{},
},
},
{
name: "create slice from endpoints with labels and annotations triggertime",
tweakEndpoint: func(ep *corev1.Endpoints) {
labels := map[string]string{"foo": "bar"}
ep.Labels = labels
annotations := map[string]string{
"foo2": "bar2",
corev1.EndpointsLastChangeTriggerTime: "date",
}
ep.Annotations = annotations
},
expectedSlice: discovery.EndpointSlice{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
"foo": "bar",
discovery.LabelServiceName: endpoints.Name,
discovery.LabelManagedBy: controllerName,
},
Annotations: map[string]string{"foo2": "bar2"},
GenerateName: fmt.Sprintf("%s-", endpoints.Name),
Namespace: endpoints.Namespace,
OwnerReferences: []metav1.OwnerReference{*ownerRef},
},
Ports: ports,
AddressType: addrType,
Endpoints: []discovery.Endpoint{},
},
},
Ports: ports,
AddressType: addrType,
Endpoints: []discovery.Endpoint{},
}
generatedSlice := newEndpointSlice(&endpoints, ports, addrType, "")

assert.EqualValues(t, expectedSlice, *generatedSlice)

if len(endpoints.Labels) > 1 {
t.Errorf("Expected Endpoints labels to not be modified, got %+v", endpoints.Labels)
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
ep := endpoints.DeepCopy()
tc.tweakEndpoint(ep)
generatedSlice := newEndpointSlice(ep, ports, addrType, "")
assert.EqualValues(t, tc.expectedSlice, *generatedSlice)
if len(endpoints.Labels) > 1 {
t.Errorf("Expected Endpoints labels to not be modified, got %+v", endpoints.Labels)
}
})
}
}

Expand Down
1 change: 1 addition & 0 deletions test/integration/endpointslice/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ go_test(
"//pkg/controller/endpointslicemirroring:go_default_library",
"//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/api/discovery/v1beta1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library",
"//staging/src/k8s.io/client-go/informers:go_default_library",
Expand Down
Loading

0 comments on commit a53e27f

Please sign in to comment.