Skip to content

Commit f8ca644

Browse files
zhiying-linzhiying-lin
authored
fix: revert serviceExport availability check (#1090)
* Revert "address comments to fix bug (#1080)" This reverts commit 9a376db. * Revert "feat: add svc export trackability (#1061)" This reverts commit 22dc1a0.
1 parent 4620852 commit f8ca644

File tree

5 files changed

+3
-341
lines changed

5 files changed

+3
-341
lines changed

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ require (
1919
github.com/spf13/pflag v1.0.5
2020
github.com/stretchr/testify v1.10.0
2121
github.com/wI2L/jsondiff v0.6.0
22-
go.goms.io/fleet-networking v0.3.5
22+
go.goms.io/fleet-networking v0.3.3
2323
go.uber.org/atomic v1.11.0
2424
go.uber.org/zap v1.27.0
2525
golang.org/x/exp v0.0.0-20241004190924-225e2abe05e6

‎go.sum‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -253,8 +253,8 @@ github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcY
253253
github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
254254
github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
255255
github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
256-
go.goms.io/fleet-networking v0.3.5 h1:nR0LFEbpu9cfBwYFVV8cGB8OARemIwtzaaFpRpp1GtQ=
257-
go.goms.io/fleet-networking v0.3.5/go.mod h1:zegH1iEZScjUWiGnXL67D/ZfWTM9DBUaSo1gPWaxLek=
256+
go.goms.io/fleet-networking v0.3.3 h1:5rwBntaUoLF+E1CzaWAEL4GdvLJPQorKhjgkbLlllPE=
257+
go.goms.io/fleet-networking v0.3.3/go.mod h1:Qgbi8M1fGaz/p5rtb6HJPmTDATWRnMt9HD1gz57WKUc=
258258
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.53.0 h1:4K4tsIXefpVJtvA/8srF4V4y0akAoPHkIslgAkjixJA=
259259
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.53.0/go.mod h1:jjdQuTGVsXV4vSs+CJ2qYDeDPf9yIJV23qlIzBm73Vg=
260260
go.opentelemetry.io/otel v1.31.0 h1:NsJcKPIW0D0H3NgzPDHmo0WW6SptzPdqg/L1zsIm2hY=

‎pkg/controllers/workapplier/availability_tracker.go‎

Lines changed: 0 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -14,18 +14,13 @@ import (
1414
policyv1 "k8s.io/api/policy/v1"
1515
apiextensionshelpers "k8s.io/apiextensions-apiserver/pkg/apihelpers"
1616
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
17-
"k8s.io/apimachinery/pkg/api/meta"
1817
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
1918
"k8s.io/apimachinery/pkg/runtime"
2019
"k8s.io/apimachinery/pkg/runtime/schema"
2120
"k8s.io/component-helpers/apps/poddisruptionbudget"
2221
"k8s.io/klog/v2"
2322

24-
fleetnetworkingv1alpha1 "go.goms.io/fleet-networking/api/v1alpha1"
25-
"go.goms.io/fleet-networking/pkg/common/objectmeta"
26-
2723
"go.goms.io/fleet/pkg/utils"
28-
"go.goms.io/fleet/pkg/utils/condition"
2924
"go.goms.io/fleet/pkg/utils/controller"
3025
)
3126

@@ -88,8 +83,6 @@ func trackInMemberClusterObjAvailabilityByGVR(
8883
return trackCRDAvailability(inMemberClusterObj)
8984
case utils.PodDisruptionBudgetGVR:
9085
return trackPDBAvailability(inMemberClusterObj)
91-
case utils.ServiceExportGVR:
92-
return trackServiceExportAvailability(inMemberClusterObj)
9386
default:
9487
if isDataResource(*gvr) {
9588
klog.V(2).InfoS("The object from the member cluster is a data object, consider it to be immediately available",
@@ -254,44 +247,6 @@ func trackPDBAvailability(curObj *unstructured.Unstructured) (ManifestProcessing
254247
return ManifestProcessingAvailabilityResultTypeNotYetAvailable, nil
255248
}
256249

257-
// trackServiceExportAvailability tracks the availability of a service export in the member cluster.
258-
// It is available if the ServiceExportValid condition is true (will be false if annotation value is invalid).
259-
// If the weight is not 0, ServiceExportValid condition must be true and the ServiceExportConflict condition must be false.
260-
func trackServiceExportAvailability(curObj *unstructured.Unstructured) (ManifestProcessingAvailabilityResultType, error) {
261-
var svcExport fleetnetworkingv1alpha1.ServiceExport
262-
if err := runtime.DefaultUnstructuredConverter.FromUnstructured(curObj.Object, &svcExport); err != nil {
263-
return ManifestProcessingAvailabilityResultTypeFailed, controller.NewUnexpectedBehaviorError(err)
264-
}
265-
266-
// Check if ServiceExport is valid and up to date
267-
svcExportObj := klog.KObj(curObj)
268-
validCond := meta.FindStatusCondition(svcExport.Status.Conditions, string(fleetnetworkingv1alpha1.ServiceExportValid))
269-
if !condition.IsConditionStatusTrue(validCond, svcExport.Generation) {
270-
klog.V(2).InfoS("Still need to wait for ServiceExport to be valid", "serviceExport", svcExportObj, "validCondition", validCond)
271-
return ManifestProcessingAvailabilityResultTypeNotYetAvailable, nil
272-
}
273-
// Validate annotation weight. Updating the annotation won't change the object generation,
274-
// so the current status is not reliable and need to validate the annotation again here.
275-
weight, err := objectmeta.ExtractWeightFromServiceExport(&svcExport)
276-
if err != nil {
277-
klog.Error(err, "ServiceExport has invalid weight", "serviceExport", svcExportObj)
278-
return ManifestProcessingAvailabilityResultTypeNotYetAvailable, nil
279-
}
280-
if weight != 0 {
281-
// Check conflict condition for non-zero weight
282-
conflictCond := meta.FindStatusCondition(svcExport.Status.Conditions, string(fleetnetworkingv1alpha1.ServiceExportConflict))
283-
if !condition.IsConditionStatusFalse(conflictCond, svcExport.Generation) {
284-
klog.V(2).InfoS("Still need to wait for ServiceExport to not have conflicts", "serviceExport", svcExportObj, "conflictCondition", conflictCond)
285-
return ManifestProcessingAvailabilityResultTypeNotYetAvailable, nil
286-
}
287-
} else {
288-
klog.V(2).InfoS("Skipping checking the conflict condition for the weight 0", "serviceExport", svcExportObj)
289-
}
290-
291-
klog.V(2).InfoS("ServiceExport is available", "serviceExport", svcExportObj)
292-
return ManifestProcessingAvailabilityResultTypeAvailable, nil
293-
}
294-
295250
// isDataResource checks if the resource is a data resource; such resources are
296251
// available immediately after creation.
297252
func isDataResource(gvr schema.GroupVersionResource) bool {

‎pkg/controllers/workapplier/availability_tracker_test.go‎

Lines changed: 0 additions & 287 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,6 @@ import (
2626
"k8s.io/klog/v2"
2727
"k8s.io/utils/ptr"
2828

29-
fleetnetworkingv1alpha1 "go.goms.io/fleet-networking/api/v1alpha1"
30-
"go.goms.io/fleet-networking/pkg/common/objectmeta"
31-
3229
fleetv1beta1 "go.goms.io/fleet/apis/placement/v1beta1"
3330
"go.goms.io/fleet/pkg/utils"
3431
"go.goms.io/fleet/pkg/utils/parallelizer"
@@ -986,290 +983,6 @@ func TestTrackInMemberClusterObjAvailabilityByGVR(t *testing.T) {
986983
}
987984
}
988985

989-
func TestServiceExportAvailability(t *testing.T) {
990-
svcExportTemplate := &fleetnetworkingv1alpha1.ServiceExport{
991-
ObjectMeta: metav1.ObjectMeta{
992-
Name: "test-svcExport",
993-
Namespace: nsName,
994-
Annotations: map[string]string{},
995-
Generation: 3,
996-
},
997-
}
998-
999-
testCases := []struct {
1000-
name string
1001-
weight string
1002-
status fleetnetworkingv1alpha1.ServiceExportStatus
1003-
wantManifestProcessingAvailabilityResultType ManifestProcessingAvailabilityResultType
1004-
}{
1005-
{
1006-
name: "available svcExport (annotation weight is 0)",
1007-
weight: "0",
1008-
status: fleetnetworkingv1alpha1.ServiceExportStatus{
1009-
Conditions: []metav1.Condition{
1010-
{
1011-
Type: string(fleetnetworkingv1alpha1.ServiceExportValid),
1012-
Status: metav1.ConditionTrue,
1013-
Reason: "ServiceIsValid",
1014-
ObservedGeneration: 3,
1015-
},
1016-
},
1017-
},
1018-
wantManifestProcessingAvailabilityResultType: ManifestProcessingAvailabilityResultTypeAvailable,
1019-
},
1020-
{
1021-
name: "unavailable svcExport (ServiceExportValid is false)",
1022-
weight: "0",
1023-
status: fleetnetworkingv1alpha1.ServiceExportStatus{
1024-
Conditions: []metav1.Condition{
1025-
{
1026-
Type: string(fleetnetworkingv1alpha1.ServiceExportValid),
1027-
Status: metav1.ConditionFalse,
1028-
Reason: "ServiceNotFound",
1029-
ObservedGeneration: 3,
1030-
},
1031-
},
1032-
},
1033-
wantManifestProcessingAvailabilityResultType: ManifestProcessingAvailabilityResultTypeNotYetAvailable,
1034-
},
1035-
{
1036-
name: "unavailable svcExport (different generation, annotation weight is 0)",
1037-
weight: "0",
1038-
status: fleetnetworkingv1alpha1.ServiceExportStatus{
1039-
Conditions: []metav1.Condition{
1040-
{
1041-
Type: string(fleetnetworkingv1alpha1.ServiceExportValid),
1042-
Status: metav1.ConditionTrue,
1043-
Reason: "ServiceIsValid",
1044-
ObservedGeneration: 2,
1045-
},
1046-
},
1047-
},
1048-
wantManifestProcessingAvailabilityResultType: ManifestProcessingAvailabilityResultTypeNotYetAvailable,
1049-
},
1050-
{
1051-
name: "available svcExport with no conflict (annotation weight is 1)",
1052-
weight: "1",
1053-
status: fleetnetworkingv1alpha1.ServiceExportStatus{
1054-
Conditions: []metav1.Condition{
1055-
{
1056-
Type: string(fleetnetworkingv1alpha1.ServiceExportValid),
1057-
Status: metav1.ConditionTrue,
1058-
Reason: "ServiceIsValid",
1059-
ObservedGeneration: 3,
1060-
},
1061-
{
1062-
Type: string(fleetnetworkingv1alpha1.ServiceExportConflict),
1063-
Status: metav1.ConditionFalse,
1064-
Reason: "NoConflictFound",
1065-
ObservedGeneration: 3,
1066-
},
1067-
},
1068-
},
1069-
wantManifestProcessingAvailabilityResultType: ManifestProcessingAvailabilityResultTypeAvailable,
1070-
},
1071-
{
1072-
name: "unavailable svcExport with conflict (annotation weight is 1)",
1073-
weight: "1",
1074-
status: fleetnetworkingv1alpha1.ServiceExportStatus{
1075-
Conditions: []metav1.Condition{
1076-
{
1077-
Type: string(fleetnetworkingv1alpha1.ServiceExportValid),
1078-
Status: metav1.ConditionTrue,
1079-
Reason: "ServiceIsValid",
1080-
ObservedGeneration: 3,
1081-
},
1082-
{
1083-
Type: string(fleetnetworkingv1alpha1.ServiceExportConflict),
1084-
Status: metav1.ConditionTrue,
1085-
Reason: "ConflictFound",
1086-
ObservedGeneration: 3,
1087-
},
1088-
},
1089-
},
1090-
wantManifestProcessingAvailabilityResultType: ManifestProcessingAvailabilityResultTypeNotYetAvailable,
1091-
},
1092-
{
1093-
name: "unavailable invalid svcExport (annotation weight is 1)",
1094-
weight: "1",
1095-
status: fleetnetworkingv1alpha1.ServiceExportStatus{
1096-
Conditions: []metav1.Condition{
1097-
{
1098-
Type: string(fleetnetworkingv1alpha1.ServiceExportValid),
1099-
Status: metav1.ConditionFalse,
1100-
Reason: "ServiceIneligible",
1101-
ObservedGeneration: 3,
1102-
},
1103-
},
1104-
},
1105-
wantManifestProcessingAvailabilityResultType: ManifestProcessingAvailabilityResultTypeNotYetAvailable,
1106-
},
1107-
{
1108-
name: "unavailable svcExport (different generation, annotation weight is 1)",
1109-
weight: "1",
1110-
status: fleetnetworkingv1alpha1.ServiceExportStatus{
1111-
Conditions: []metav1.Condition{
1112-
{
1113-
Type: string(fleetnetworkingv1alpha1.ServiceExportValid),
1114-
Status: metav1.ConditionTrue,
1115-
Reason: "ServiceIsValid",
1116-
ObservedGeneration: 3,
1117-
},
1118-
{
1119-
Type: string(fleetnetworkingv1alpha1.ServiceExportConflict),
1120-
Status: metav1.ConditionTrue,
1121-
Reason: "ConflictFound",
1122-
ObservedGeneration: 2,
1123-
},
1124-
},
1125-
},
1126-
wantManifestProcessingAvailabilityResultType: ManifestProcessingAvailabilityResultTypeNotYetAvailable,
1127-
},
1128-
{
1129-
name: "unavailable svcExport (no annotation weight, no conflict condition)",
1130-
weight: "",
1131-
status: fleetnetworkingv1alpha1.ServiceExportStatus{
1132-
Conditions: []metav1.Condition{
1133-
{
1134-
Type: string(fleetnetworkingv1alpha1.ServiceExportValid),
1135-
Status: metav1.ConditionTrue,
1136-
Reason: "ServiceIsValid",
1137-
ObservedGeneration: 3,
1138-
},
1139-
},
1140-
},
1141-
wantManifestProcessingAvailabilityResultType: ManifestProcessingAvailabilityResultTypeNotYetAvailable,
1142-
},
1143-
{
1144-
name: "available svcExport (no annotation weight)",
1145-
weight: "",
1146-
status: fleetnetworkingv1alpha1.ServiceExportStatus{
1147-
Conditions: []metav1.Condition{
1148-
{
1149-
Type: string(fleetnetworkingv1alpha1.ServiceExportValid),
1150-
Status: metav1.ConditionTrue,
1151-
Reason: "ServiceIsValid",
1152-
ObservedGeneration: 3,
1153-
},
1154-
{
1155-
Type: string(fleetnetworkingv1alpha1.ServiceExportConflict),
1156-
Status: metav1.ConditionFalse,
1157-
Reason: "NoConflictFound",
1158-
ObservedGeneration: 3,
1159-
},
1160-
},
1161-
},
1162-
wantManifestProcessingAvailabilityResultType: ManifestProcessingAvailabilityResultTypeAvailable,
1163-
},
1164-
{
1165-
name: "unavailable svcExport (no annotation weight with conflict)",
1166-
weight: "",
1167-
status: fleetnetworkingv1alpha1.ServiceExportStatus{
1168-
Conditions: []metav1.Condition{
1169-
{
1170-
Type: string(fleetnetworkingv1alpha1.ServiceExportValid),
1171-
Status: metav1.ConditionTrue,
1172-
Reason: "ServiceIsValid",
1173-
ObservedGeneration: 3,
1174-
},
1175-
{
1176-
Type: string(fleetnetworkingv1alpha1.ServiceExportConflict),
1177-
Status: metav1.ConditionTrue,
1178-
Reason: "ConflictFound",
1179-
ObservedGeneration: 3,
1180-
},
1181-
},
1182-
},
1183-
wantManifestProcessingAvailabilityResultType: ManifestProcessingAvailabilityResultTypeNotYetAvailable,
1184-
},
1185-
{
1186-
name: "unavailable svcExport (no annotation weight, different generation)",
1187-
weight: "",
1188-
status: fleetnetworkingv1alpha1.ServiceExportStatus{
1189-
Conditions: []metav1.Condition{
1190-
{
1191-
Type: string(fleetnetworkingv1alpha1.ServiceExportValid),
1192-
Status: metav1.ConditionTrue,
1193-
Reason: "ServiceIsValid",
1194-
ObservedGeneration: 3,
1195-
},
1196-
{
1197-
Type: string(fleetnetworkingv1alpha1.ServiceExportConflict),
1198-
Status: metav1.ConditionFalse,
1199-
Reason: "NoConflictFound",
1200-
ObservedGeneration: 2,
1201-
},
1202-
},
1203-
},
1204-
wantManifestProcessingAvailabilityResultType: ManifestProcessingAvailabilityResultTypeNotYetAvailable,
1205-
},
1206-
{
1207-
name: "unavailable svcExport (no annotation weight, invalid service export)",
1208-
weight: "",
1209-
status: fleetnetworkingv1alpha1.ServiceExportStatus{
1210-
Conditions: []metav1.Condition{
1211-
{
1212-
Type: string(fleetnetworkingv1alpha1.ServiceExportValid),
1213-
Status: metav1.ConditionFalse,
1214-
Reason: "ServiceIsNotValid",
1215-
ObservedGeneration: 3,
1216-
},
1217-
},
1218-
},
1219-
wantManifestProcessingAvailabilityResultType: ManifestProcessingAvailabilityResultTypeNotYetAvailable,
1220-
},
1221-
{
1222-
name: "unavailable svcExport (invalid weight)",
1223-
weight: "a",
1224-
status: fleetnetworkingv1alpha1.ServiceExportStatus{
1225-
Conditions: []metav1.Condition{
1226-
{
1227-
Type: string(fleetnetworkingv1alpha1.ServiceExportValid),
1228-
Status: metav1.ConditionTrue,
1229-
Reason: "ServiceIsValid",
1230-
ObservedGeneration: 3,
1231-
},
1232-
},
1233-
},
1234-
wantManifestProcessingAvailabilityResultType: ManifestProcessingAvailabilityResultTypeNotYetAvailable,
1235-
},
1236-
{
1237-
name: "unavailable svcExport (out of range weight)",
1238-
weight: "1002",
1239-
status: fleetnetworkingv1alpha1.ServiceExportStatus{
1240-
Conditions: []metav1.Condition{
1241-
{
1242-
Type: string(fleetnetworkingv1alpha1.ServiceExportValid),
1243-
Status: metav1.ConditionTrue,
1244-
Reason: "ServiceIsValid",
1245-
ObservedGeneration: 3,
1246-
},
1247-
},
1248-
},
1249-
wantManifestProcessingAvailabilityResultType: ManifestProcessingAvailabilityResultTypeNotYetAvailable,
1250-
},
1251-
}
1252-
1253-
for _, tc := range testCases {
1254-
t.Run(tc.name, func(t *testing.T) {
1255-
svcExport := svcExportTemplate.DeepCopy()
1256-
if tc.weight != "" {
1257-
svcExport.Annotations[objectmeta.ServiceExportAnnotationWeight] = tc.weight
1258-
}
1259-
svcExport.Status = tc.status
1260-
gotResTyp, err := trackServiceExportAvailability(toUnstructured(t, svcExport))
1261-
if err != nil {
1262-
t.Fatalf("trackServiceExportAvailability() = %v, want no error", err)
1263-
}
1264-
1265-
// Check the result type
1266-
if gotResTyp != tc.wantManifestProcessingAvailabilityResultType {
1267-
t.Errorf("manifestProcessingAvailabilityResultType = %v, want %v", gotResTyp, tc.wantManifestProcessingAvailabilityResultType)
1268-
}
1269-
})
1270-
}
1271-
}
1272-
1273986
// TestTrackInMemberClusterObjAvailability tests the trackInMemberClusterObjAvailability method.
1274987
func TestTrackInMemberClusterObjAvailability(t *testing.T) {
1275988
ctx := context.Background()

0 commit comments

Comments
 (0)