last-applied-config annotation no longer mirrored to endpoint slices

Handles incorrect mirroring of endpoint annotations to created endpoint
slices, specifically the last-applied-config. Also updates tests
and adds test cases for the same
This commit is contained in:
Dakshraj Sharma 2021-06-09 18:55:48 +05:30
parent 8eef6438e6
commit 211485c23d
3 changed files with 137 additions and 5 deletions

View File

@ -197,8 +197,8 @@ func (r *reconciler) reconcileByPortMapping(
// if >0 existing slices, mark all but 1 for deletion.
slices.toDelete = existingSlices[1:]
// generated slices must mirror all endpoints annotations but EndpointsLastChangeTriggerTime
compareAnnotations := cloneAndRemoveKeys(endpoints.Annotations, corev1.EndpointsLastChangeTriggerTime)
// generated slices must mirror all endpoints annotations but EndpointsLastChangeTriggerTime and LastAppliedConfigAnnotation
compareAnnotations := cloneAndRemoveKeys(endpoints.Annotations, corev1.EndpointsLastChangeTriggerTime, corev1.LastAppliedConfigAnnotation)
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)

View File

@ -44,6 +44,7 @@ func TestReconcile(t *testing.T) {
testName string
subsets []corev1.EndpointSubset
epLabels map[string]string
epAnnotations map[string]string
endpointsDeletionPending bool
maxEndpointsPerSubset int32
existingEndpointSlices []*discovery.EndpointSlice
@ -791,6 +792,118 @@ func TestReconcile(t *testing.T) {
expectedClientActions: 2,
maxEndpointsPerSubset: 2,
expectedMetrics: &expectedMetrics{desiredSlices: 2, actualSlices: 2, desiredEndpoints: 4, addedPerSync: 4, updatedPerSync: 0, removedPerSync: 0, skippedPerSync: 1, numCreated: 2, numUpdated: 0},
}, {
testName: "The last-applied-configuration annotation should not get mirrored to created or updated endpoint slices",
epAnnotations: map[string]string{
corev1.LastAppliedConfigAnnotation: "{\"apiVersion\":\"v1\",\"kind\":\"Endpoints\",\"subsets\":[]}",
},
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{},
expectedNumSlices: 1,
expectedClientActions: 1,
expectedMetrics: &expectedMetrics{addedPerSync: 1, numCreated: 1, desiredEndpoints: 1, desiredSlices: 1, actualSlices: 1},
}, {
testName: "The last-applied-configuration annotation shouldn't get added to created endpoint slices",
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{},
expectedNumSlices: 1,
expectedClientActions: 1,
expectedMetrics: &expectedMetrics{addedPerSync: 1, numCreated: 1, desiredEndpoints: 1, desiredSlices: 1, actualSlices: 1},
}, {
testName: "The last-applied-configuration shouldn't get mirrored to endpoint slices when it's value is empty",
epAnnotations: map[string]string{
corev1.LastAppliedConfigAnnotation: "",
},
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{},
expectedNumSlices: 1,
expectedClientActions: 1,
expectedMetrics: &expectedMetrics{addedPerSync: 1, numCreated: 1, desiredEndpoints: 1, desiredSlices: 1, actualSlices: 1},
}, {
testName: "Annotations other than last-applied-configuration should get correctly mirrored",
epAnnotations: map[string]string{
corev1.LastAppliedConfigAnnotation: "{\"apiVersion\":\"v1\",\"kind\":\"Endpoints\",\"subsets\":[]}",
"foo": "bar",
},
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{},
expectedNumSlices: 1,
expectedClientActions: 1,
expectedMetrics: &expectedMetrics{addedPerSync: 1, numCreated: 1, desiredEndpoints: 1, desiredSlices: 1, actualSlices: 1},
}, {
testName: "Annotation mirroring should remove the last-applied-configuration annotation from existing endpoint slices",
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{
corev1.LastAppliedConfigAnnotation: "{\"apiVersion\":\"v1\",\"kind\":\"Endpoints\",\"subsets\":[]}",
},
},
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,
}}
for _, tc := range testCases {
@ -799,7 +912,7 @@ func TestReconcile(t *testing.T) {
setupMetrics()
namespace := "test"
endpoints := corev1.Endpoints{
ObjectMeta: metav1.ObjectMeta{Name: "test-ep", Namespace: namespace, Labels: tc.epLabels},
ObjectMeta: metav1.ObjectMeta{Name: "test-ep", Namespace: namespace, Labels: tc.epLabels, Annotations: tc.epAnnotations},
Subsets: tc.subsets,
}
@ -880,6 +993,25 @@ func expectEndpointSlices(t *testing.T, num, maxEndpointsPerSubset int, endpoint
if serviceNameVal != endpoints.Name {
t.Errorf("Expected EndpointSlice to have %s label set to %s, got %s", discovery.LabelServiceName, endpoints.Name, serviceNameVal)
}
_, ok = epSlice.Annotations[corev1.LastAppliedConfigAnnotation]
if ok {
t.Errorf("Expected LastAppliedConfigAnnotation to be unset, got %s", epSlice.Annotations[corev1.LastAppliedConfigAnnotation])
}
_, ok = epSlice.Annotations[corev1.EndpointsLastChangeTriggerTime]
if ok {
t.Errorf("Expected EndpointsLastChangeTriggerTime to be unset, got %s", epSlice.Annotations[corev1.EndpointsLastChangeTriggerTime])
}
for annotation, val := range endpoints.Annotations {
if annotation == corev1.EndpointsLastChangeTriggerTime || annotation == corev1.LastAppliedConfigAnnotation {
continue
}
if epSlice.Annotations[annotation] != val {
t.Errorf("Expected endpoint annotation %s to be mirrored correctly, got %s", annotation, epSlice.Annotations[annotation])
}
}
}
for _, epSubset := range endpoints.Subsets {

View File

@ -87,9 +87,9 @@ func newEndpointSlice(endpoints *corev1.Endpoints, ports []discovery.EndpointPor
epSlice.Labels[discovery.LabelServiceName] = endpoints.Name
epSlice.Labels[discovery.LabelManagedBy] = controllerName
// clone all annotations but EndpointsLastChangeTriggerTime
// clone all annotations but EndpointsLastChangeTriggerTime and LastAppliedConfigAnnotation
for annotation, val := range endpoints.Annotations {
if annotation == corev1.EndpointsLastChangeTriggerTime {
if annotation == corev1.EndpointsLastChangeTriggerTime || annotation == corev1.LastAppliedConfigAnnotation {
continue
}
epSlice.Annotations[annotation] = val