diff --git a/internal/controller/gatewayclass_congroller.go b/internal/controller/gatewayclass_congroller.go index 82e4ee1b0..26578cf64 100644 --- a/internal/controller/gatewayclass_congroller.go +++ b/internal/controller/gatewayclass_congroller.go @@ -3,18 +3,26 @@ package controller import ( "context" "fmt" + "time" "github.com/go-logr/logr" meta "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/predicate" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" "github.com/api7/api7-ingress-controller/internal/controller/config" ) +const ( + FinalizerGatewayClassProtection = "apisix.apache.org/gc-protection" +) + // +kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=gatewayclasses,verbs=get;list;watch;update // +kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=gatewayclasses/status,verbs=get;update @@ -23,11 +31,13 @@ type GatewayClassReconciler struct { //nolint:revive client.Client Scheme *runtime.Scheme + record.EventRecorder Log logr.Logger } // SetupWithManager sets up the controller with the Manager. func (r *GatewayClassReconciler) SetupWithManager(mgr ctrl.Manager) error { + r.EventRecorder = mgr.GetEventRecorderFor("gatewayclass-controller") return ctrl.NewControllerManagedBy(mgr). For(&gatewayv1.GatewayClass{}). WithEventFilter(predicate.NewPredicateFuncs(r.GatewayClassFilter)). @@ -41,6 +51,43 @@ func (r *GatewayClassReconciler) Reconcile(ctx context.Context, req ctrl.Request return ctrl.Result{}, client.IgnoreNotFound(err) } + if gc.GetDeletionTimestamp().IsZero() { + if !controllerutil.ContainsFinalizer(gc, FinalizerGatewayClassProtection) { + controllerutil.AddFinalizer(gc, FinalizerGatewayClassProtection) + if err := r.Update(ctx, gc); err != nil { + return ctrl.Result{}, err + } + } + } else { + if controllerutil.ContainsFinalizer(gc, FinalizerGatewayClassProtection) { + var gatewayList gatewayv1.GatewayList + if err := r.List(ctx, &gatewayList); err != nil { + r.Log.Error(err, "failed to list gateways") + return ctrl.Result{}, err + } + var gateways []types.NamespacedName + for _, gateway := range gatewayList.Items { + if string(gateway.Spec.GatewayClassName) == gc.GetName() { + gateways = append(gateways, types.NamespacedName{ + Namespace: gateway.GetNamespace(), + Name: gateway.GetName(), + }) + } + } + if len(gateways) > 0 { + r.Eventf(gc, "Warning", "DeletionBlocked", "the GatewayClass is still used by Gateways: %v", gateways) + return ctrl.Result{RequeueAfter: 5 * time.Second}, nil + } else { + controllerutil.RemoveFinalizer(gc, FinalizerGatewayClassProtection) + if err := r.Update(ctx, gc); err != nil { + return ctrl.Result{}, err + } + } + } + + return ctrl.Result{}, nil + } + condition := meta.Condition{ Type: string(gatewayv1.GatewayClassConditionStatusAccepted), Status: meta.ConditionTrue, diff --git a/test/e2e/gatewayapi/gateway.go b/test/e2e/gatewayapi/gateway.go index 6c885dfa9..fc93ac306 100644 --- a/test/e2e/gatewayapi/gateway.go +++ b/test/e2e/gatewayapi/gateway.go @@ -112,7 +112,7 @@ spec: Expect(gcyaml).To(ContainSubstring("message: the gatewayclass has been accepted by the api7-ingress-controller"), "checking GatewayClass condition message") By("create Gateway") - err = s.CreateResourceFromString(defautlGateway) + err = s.CreateResourceFromStringWithNamespace(defautlGateway, s.CurrentNamespace()) Expect(err).NotTo(HaveOccurred(), "creating Gateway") time.Sleep(5 * time.Second) @@ -123,7 +123,7 @@ spec: Expect(gwyaml).To(ContainSubstring("message: the gateway has been accepted by the api7-ingress-controller"), "checking Gateway condition message") By("create Gateway with not accepted GatewayClass") - err = s.CreateResourceFromString(noClassGateway) + err = s.CreateResourceFromStringWithNamespace(noClassGateway, s.CurrentNamespace()) Expect(err).NotTo(HaveOccurred(), "creating Gateway") time.Sleep(5 * time.Second) @@ -184,7 +184,7 @@ spec: time.Sleep(5 * time.Second) By("create Gateway") - err = s.CreateResourceFromString(defaultGateway) + err = s.CreateResourceFromStringWithNamespace(defaultGateway, s.CurrentNamespace()) Expect(err).NotTo(HaveOccurred(), "creating Gateway") time.Sleep(10 * time.Second) @@ -257,7 +257,7 @@ spec: time.Sleep(5 * time.Second) By("create Gateway") - err = s.CreateResourceFromString(defaultGateway) + err = s.CreateResourceFromStringWithNamespace(defaultGateway, s.CurrentNamespace()) Expect(err).NotTo(HaveOccurred(), "creating Gateway") time.Sleep(10 * time.Second) diff --git a/test/e2e/gatewayapi/gatewayclass.go b/test/e2e/gatewayapi/gatewayclass.go index 4bde9a9f7..a68b1eed2 100644 --- a/test/e2e/gatewayapi/gatewayclass.go +++ b/test/e2e/gatewayapi/gatewayclass.go @@ -31,6 +31,18 @@ metadata: name: api7-not-accepeted spec: controllerName: "apisix.apache.org/not-exist" +` + const defaultGateway = ` +apiVersion: gateway.networking.k8s.io/v1 +kind: Gateway +metadata: + name: api7ee +spec: + gatewayClassName: api7 + listeners: + - name: http1 + protocol: HTTP + port: 80 ` It("Create GatewayClass", func() { By("create default GatewayClass") @@ -53,5 +65,50 @@ spec: Expect(gcyaml).To(ContainSubstring(`status: Unknown`), "checking GatewayClass condition status") Expect(gcyaml).To(ContainSubstring("message: Waiting for controller"), "checking GatewayClass condition message") }) + + It("Delete GatewayClass", func() { + By("create default GatewayClass") + err := s.CreateResourceFromStringWithNamespace(defautlGatewayClass, "") + Expect(err).NotTo(HaveOccurred(), "creating GatewayClass") + Eventually(func() string { + spec, err := s.GetResourceYaml("GatewayClass", "api7") + Expect(err).NotTo(HaveOccurred(), "get resource yaml") + return spec + }).WithTimeout(8 * time.Second).ProbeEvery(time.Second).Should(ContainSubstring(`status: "True"`)) + + By("create a Gateway") + err = s.CreateResourceFromStringWithNamespace(defaultGateway, s.CurrentNamespace()) + Expect(err).NotTo(HaveOccurred(), "creating Gateway") + time.Sleep(time.Second) + + By("try to delete the GatewayClass") + _, err = s.RunKubectlAndGetOutput("delete", "GatewayClass", "api7", "--wait=false") + Expect(err).NotTo(HaveOccurred()) + + _, err = s.GetResourceYaml("GatewayClass", "api7") + Expect(err).NotTo(HaveOccurred(), "get resource yaml") + + output, err := s.RunKubectlAndGetOutput("describe", "GatewayClass", "api7") + Expect(err).NotTo(HaveOccurred(), "describe GatewayClass api7") + Expect(output).To(And( + ContainSubstring("Warning"), + ContainSubstring("DeletionBlocked"), + ContainSubstring("gatewayclass-controller"), + ContainSubstring("the GatewayClass is still used by Gateways"), + )) + + By("delete the Gateway") + err = s.DeleteResource("Gateway", "api7ee") + Expect(err).NotTo(HaveOccurred(), "deleting Gateway") + time.Sleep(time.Second) + + By("try to delete the GatewayClass again") + err = s.DeleteResource("GatewayClass", "api7") + Expect(err).NotTo(HaveOccurred()) + + _, err = s.GetResourceYaml("GatewayClass", "api7") + Expect(err).To(HaveOccurred(), "get resource yaml") + Expect(err.Error()).To(ContainSubstring("not found")) + }) }) }) diff --git a/test/e2e/gatewayapi/gatewayproxy.go b/test/e2e/gatewayapi/gatewayproxy.go index 434d27efa..87ce7e47a 100644 --- a/test/e2e/gatewayapi/gatewayproxy.go +++ b/test/e2e/gatewayapi/gatewayproxy.go @@ -207,7 +207,7 @@ spec: time.Sleep(5 * time.Second) By("Create Gateway with GatewayProxy") - err = s.CreateResourceFromString(fmt.Sprintf(gatewayWithProxy, gatewayClassName)) + err = s.CreateResourceFromStringWithNamespace(fmt.Sprintf(gatewayWithProxy, gatewayClassName), s.CurrentNamespace()) Expect(err).NotTo(HaveOccurred(), "creating Gateway with GatewayProxy") time.Sleep(5 * time.Second) @@ -220,9 +220,9 @@ spec: AfterEach(func() { By("Clean up resources") - _ = s.DeleteResourceFromString(fmt.Sprintf(gatewayProxyWithEnabledPlugin, framework.DashboardTLSEndpoint, s.AdminKey())) _ = s.DeleteResourceFromString(fmt.Sprintf(httpRouteForTest, "api7")) _ = s.DeleteResourceFromString(fmt.Sprintf(gatewayWithProxy, gatewayClassName)) + _ = s.DeleteResourceFromString(fmt.Sprintf(gatewayProxyWithEnabledPlugin, framework.DashboardTLSEndpoint, s.AdminKey())) }) Context("Test Gateway with enabled GatewayProxy plugin", func() { diff --git a/test/e2e/gatewayapi/httproute.go b/test/e2e/gatewayapi/httproute.go index f2b2ea353..c6d2099b7 100644 --- a/test/e2e/gatewayapi/httproute.go +++ b/test/e2e/gatewayapi/httproute.go @@ -125,7 +125,7 @@ spec: Expect(gcyaml).To(ContainSubstring("message: the gatewayclass has been accepted by the api7-ingress-controller"), "checking GatewayClass condition message") By("create Gateway") - err = s.CreateResourceFromString(fmt.Sprintf(defaultGateway, gatewayClassName)) + err = s.CreateResourceFromStringWithNamespace(fmt.Sprintf(defaultGateway, gatewayClassName), s.CurrentNamespace()) Expect(err).NotTo(HaveOccurred(), "creating Gateway") time.Sleep(5 * time.Second) @@ -158,7 +158,7 @@ spec: Expect(gcyaml).To(ContainSubstring("message: the gatewayclass has been accepted by the api7-ingress-controller"), "checking GatewayClass condition message") By("create Gateway") - err = s.CreateResourceFromString(fmt.Sprintf(defaultGatewayHTTPS, gatewayClassName)) + err = s.CreateResourceFromStringWithNamespace(fmt.Sprintf(defaultGatewayHTTPS, gatewayClassName), s.CurrentNamespace()) Expect(err).NotTo(HaveOccurred(), "creating Gateway") time.Sleep(5 * time.Second) diff --git a/test/e2e/scaffold/k8s.go b/test/e2e/scaffold/k8s.go index 413fe0fbd..5e0d544c5 100644 --- a/test/e2e/scaffold/k8s.go +++ b/test/e2e/scaffold/k8s.go @@ -129,6 +129,10 @@ func (s *Scaffold) DeleteResourceFromStringWithNamespace(yaml, namespace string) return k8s.KubectlDeleteFromStringE(s.t, s.kubectlOptions, yaml) } +func (s *Scaffold) CurrentNamespace() string { + return s.kubectlOptions.Namespace +} + func (s *Scaffold) NewAPISIX() (dashboard.Dashboard, error) { return dashboard.NewClient() } @@ -268,7 +272,7 @@ func (s *Scaffold) ApplyDefaultGatewayResource( ) By("create Gateway") - err = s.CreateResourceFromString(fmt.Sprintf(defaultGateway, gatewayClassName)) + err = s.CreateResourceFromStringWithNamespace(fmt.Sprintf(defaultGateway, gatewayClassName), s.CurrentNamespace()) Expect(err).NotTo(HaveOccurred(), "creating Gateway") time.Sleep(5 * time.Second) diff --git a/test/e2e/scaffold/scaffold.go b/test/e2e/scaffold/scaffold.go index 70a4fc483..d808fd4e1 100644 --- a/test/e2e/scaffold/scaffold.go +++ b/test/e2e/scaffold/scaffold.go @@ -488,8 +488,8 @@ func (s *Scaffold) afterEach() { err := k8s.DeleteNamespaceE(s.t, s.kubectlOptions, s.namespace) Expect(err).NotTo(HaveOccurred(), "deleting namespace "+s.namespace) - for _, f := range s.finalizers { - runWithRecover(f) + for i := len(s.finalizers) - 1; i >= 0; i-- { + runWithRecover(s.finalizers[i]) } // Wait for a while to prevent the worker node being overwhelming