Skip to content
This repository has been archived by the owner on Dec 16, 2024. It is now read-only.

small fix for gateways being deleted after placement deleted #579

Merged
merged 4 commits into from
Sep 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
20 changes: 10 additions & 10 deletions pkg/controllers/gateway/gateway_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
log.V(3).Info("reconciling gateway", "classname", upstreamGateway.Spec.GatewayClassName)
if isDeleting(upstreamGateway) {
log.Info("gateway being deleted ", "gateway", upstreamGateway.Name, "namespace", upstreamGateway.Namespace)
if _, _, _, err := r.reconcileDownstreamFromUpstreamGateway(ctx, upstreamGateway, nil); err != nil {
if _, _, _, err := r.reconcileDownstreamFromUpstreamGateway(ctx, upstreamGateway, nil); client.IgnoreNotFound(err) != nil {
return ctrl.Result{}, fmt.Errorf("failed to reconcile downstream gateway after upstream gateway deleted: %s ", err)
}
controllerutil.RemoveFinalizer(upstreamGateway, GatewayFinalizer)
Expand Down Expand Up @@ -379,29 +379,29 @@ func (r *GatewayReconciler) reconcileParams(_ context.Context, gateway *gatewayv
}

func buildProgrammedCondition(generation int64, placed []string, programmedStatus metav1.ConditionStatus, err error) metav1.Condition {
errorMsg := ""
if err != nil {
errorMsg = err.Error()
}
var reason = gatewayv1beta1.GatewayReasonProgrammed
message := "waiting for gateway to placed on clusters %v %s"
message := "waiting for gateway to placed on clusters %v"
if programmedStatus == metav1.ConditionTrue {
message = "gateway placed on clusters %v %s"
message = fmt.Sprintf("gateway placed on clusters %v", placed)
}
if programmedStatus == metav1.ConditionFalse {
message = "gateway failed to be placed on all clusters %v error %s"
message = fmt.Sprintf("gateway failed to be placed on all clusters %v", placed)
reason = gatewayv1beta1.GatewayReasonInvalid
}
if programmedStatus == metav1.ConditionUnknown {
message = "current state of the gateway is unknown error %s"
message = "current state of the gateway is unknown"
reason = gatewayv1beta1.GatewayReasonPending
}

if err != nil {
message += " error: " + err.Error()
}

cond := metav1.Condition{
Type: string(gatewayv1beta1.GatewayConditionProgrammed),
Status: programmedStatus,
Reason: string(reason),
Message: fmt.Sprintf(message, placed, errorMsg),
Message: message,
ObservedGeneration: generation,
}
return cond
Expand Down
4 changes: 2 additions & 2 deletions pkg/controllers/gateway/gateway_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ func Test_buildProgrammedStatus(t *testing.T) {
Status: v1.ConditionFalse,
ObservedGeneration: 1,
Reason: string(gatewayv1beta1.GatewayReasonInvalid),
Message: "gateway failed to be placed on all clusters [] error",
Message: "gateway failed to be placed on all clusters",
},
},
},
Expand All @@ -579,7 +579,7 @@ func Test_buildProgrammedStatus(t *testing.T) {
Status: v1.ConditionUnknown,
ObservedGeneration: 1,
Reason: string(gatewayv1beta1.GatewayReasonPending),
Message: "current state of the gateway is unknown error",
Message: "current state of the gateway is unknown",
},
},
},
Expand Down
4 changes: 3 additions & 1 deletion pkg/placement/ocm.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
k8smeta "k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/client-go/tools/cache"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -254,7 +255,8 @@ func (op *ocmPlacer) GetClusters(ctx context.Context, gateway *gatewayv1beta1.Ga
return targetClusters, err
}
if len(pdList.Items) == 0 {
return targetClusters, fmt.Errorf("no placemement decisions found for placement selector %s", selectedPlacement)
placementNotFound := k8serrors.NewNotFound(schema.GroupResource{Resource: "PlacementDecision"}, selectedPlacement)
return targetClusters, fmt.Errorf("no PlacementDecisions found for placement %s via label selector: %s error: %w", selectedPlacement, labelSelector, placementNotFound)
}
for _, pd := range pdList.Items {
for _, d := range pd.Status.Decisions {
Expand Down
13 changes: 8 additions & 5 deletions pkg/placement/placement_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
workv1 "open-cluster-management.io/api/work/v1"

corev1 "k8s.io/api/core/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/sets"
Expand Down Expand Up @@ -521,23 +522,25 @@ func TestGetClusters(t *testing.T) {
if err == nil {
t.Fatalf("expected an error but got none")
}
if !k8serrors.IsNotFound(err) {
t.Fatalf("expected a not found err %v", err)
}
if !got.Equal(expected) {
t.Fatalf("expected clusters %v but it was not present in %v", expected.UnsortedList(), got.UnsortedList())
}
},
PlacementDecision: func(clusters sets.Set[string]) *pd.PlacementDecision {
dec := &pd.PlacementDecision{
Status: pd.PlacementDecisionStatus{},
}
return dec
return nil
},
},
}

for _, testCase := range testCases {
t.Run(testCase.Name, func(t *testing.T) {
f := fake.NewClientBuilder()
f.WithObjects(testCase.PlacementDecision(testCase.Clusters))
if pds := testCase.PlacementDecision(testCase.Clusters); pds != nil {
f.WithObjects(pds)
}
p := placement.NewOCMPlacer(f.Build())
cs, err := p.GetClusters(context.TODO(), testCase.Gateway)
testCase.Assert(t, err, cs, testCase.Clusters)
Expand Down
9 changes: 6 additions & 3 deletions test/integration/dnspolicy_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,8 @@ var _ = Describe("DNSPolicy", Ordered, func() {

AfterEach(func() {
err := k8sClient.Delete(ctx, gateway)
Expect(err).ToNot(HaveOccurred())
// ignore not found err
Expect(client.IgnoreNotFound(err)).ToNot(HaveOccurred())
})

Context("weighted dnspolicy", func() {
Expand Down Expand Up @@ -1328,8 +1329,10 @@ var _ = Describe("DNSPolicy", Ordered, func() {
Protocol: gatewayv1beta1.HTTPProtocolType,
}

gateway.Spec.Listeners = append(gateway.Spec.Listeners, otherListener)
Expect(k8sClient.Update(ctx, gateway)).To(BeNil())
Eventually(func() error {
gateway.Spec.Listeners = append(gateway.Spec.Listeners, otherListener)
return k8sClient.Update(ctx, gateway)
}).Should(BeNil())

probeList := &v1alpha1.DNSHealthCheckProbeList{}
Eventually(func() error {
Expand Down