refactor: remove deprecated discovery and health check features from ApisixUpstream in standalone mode

Author: dspo

api/adc/types.go

@@ -197,7 +197,7 @@ type Upstream struct {
 
 	HashOn       string        `json:"hash_on,omitempty" yaml:"hash_on,omitempty"`
 	Key          string        `json:"key,omitempty" yaml:"key,omitempty"`
-	Nodes        UpstreamNodes `json:"nodes,omitempty" yaml:"nodes,omitempty"`
+	Nodes        UpstreamNodes `json:"nodes" yaml:"nodes"`
 	PassHost     string        `json:"pass_host,omitempty" yaml:"pass_host,omitempty"`
 	Retries      *int64        `json:"retries,omitempty" yaml:"retries,omitempty"`
 	RetryTimeout *float64      `json:"retry_timeout,omitempty" yaml:"retry_timeout,omitempty"`

api/v2/apisixupstream_types.go

@@ -19,7 +19,6 @@ import (
 )
 
 // ApisixUpstreamSpec describes the specification of ApisixUpstream.
-// +kubebuilder:validation:XValidation:rule="has(self.subsets) || (has(self.externalNodes)!=has(self.discovery))"
 type ApisixUpstreamSpec struct {
 	// IngressClassName is the name of an IngressClass cluster resource.
 	// controller implementations use this field to know whether they should be
@@ -95,6 +94,7 @@ type ApisixUpstreamConfig struct {
 	// +kubebuilder:validation:Optional
 	Timeout *UpstreamTimeout `json:"timeout,omitempty" yaml:"timeout,omitempty"`
 
+	// Deprecated: this is no longer support on standalone mode.
 	// The health check configurations for the upstream.
 	// +kubebuilder:validation:Optional
 	HealthCheck *HealthCheck `json:"healthCheck,omitempty" yaml:"healthCheck,omitempty"`
@@ -119,6 +119,7 @@ type ApisixUpstreamConfig struct {
 	// +kubebuilder:validation:Optional
 	UpstreamHost string `json:"upstreamHost,omitempty" yaml:"upstreamHost,omitempty"`
 
+	// Deprecated: this is no longer support on standalone mode.
 	// Discovery is used to configure service discovery for upstream.
 	// +kubebuilder:validation:Optional
 	Discovery *Discovery `json:"discovery,omitempty" yaml:"discovery,omitempty"`

config/crd/bases/apisix.apache.org_apisixupstreams.yaml

@@ -40,8 +40,9 @@ spec:
             description: ApisixUpstreamSpec describes the specification of ApisixUpstream.
             properties:
               discovery:
-                description: Discovery is used to configure service discovery for
-                  upstream.
+                description: |-
+                  Deprecated: this is no longer support on standalone mode.
+                  Discovery is used to configure service discovery for upstream.
                 properties:
                   args:
                     additionalProperties:
@@ -77,7 +78,9 @@ spec:
                 minItems: 1
                 type: array
               healthCheck:
-                description: The health check configurations for the upstream.
+                description: |-
+                  Deprecated: this is no longer support on standalone mode.
+                  The health check configurations for the upstream.
                 properties:
                   active:
                     description: ActiveHealthCheck defines the active kind of upstream
@@ -244,8 +247,9 @@ spec:
                     them if they are set on the port level.
                   properties:
                     discovery:
-                      description: Discovery is used to configure service discovery
-                        for upstream.
+                      description: |-
+                        Deprecated: this is no longer support on standalone mode.
+                        Discovery is used to configure service discovery for upstream.
                       properties:
                         args:
                           additionalProperties:
@@ -260,7 +264,9 @@ spec:
                       - type
                       type: object
                     healthCheck:
-                      description: The health check configurations for the upstream.
+                      description: |-
+                        Deprecated: this is no longer support on standalone mode.
+                        The health check configurations for the upstream.
                       properties:
                         active:
                           description: ActiveHealthCheck defines the active kind of
@@ -555,8 +561,6 @@ spec:
                   the pass_host is set to rewrite
                 type: string
             type: object
-            x-kubernetes-validations:
-            - rule: has(self.subsets) || (has(self.externalNodes)!=has(self.discovery))
           status:
             description: ApisixStatus is the status report for Apisix ingress Resources
             properties:

docs/crd/api.md

@@ -1334,12 +1334,12 @@ load balancer, health check, etc.
 | `scheme` _string_ | The scheme used to talk with the upstream. Now value can be http, grpc. |
 | `retries` _integer_ | How many times that the proxy (Apache APISIX) should do when errors occur (error, timeout or bad http status codes like 500, 502). |
 | `timeout` _[UpstreamTimeout](#upstreamtimeout)_ | Timeout settings for the read, send and connect to the upstream. |
-| `healthCheck` _[HealthCheck](#healthcheck)_ | The health check configurations for the upstream. |
+| `healthCheck` _[HealthCheck](#healthcheck)_ | Deprecated: this is no longer support on standalone mode. The health check configurations for the upstream. |
 | `tlsSecret` _[ApisixSecret](#apisixsecret)_ | Set the client certificate when connecting to TLS upstream. |
 | `subsets` _[ApisixUpstreamSubset](#apisixupstreamsubset) array_ | Subsets groups the service endpoints by their labels. Usually used to differentiate service versions. |
 | `passHost` _string_ | Configures the host when the request is forwarded to the upstream. Can be one of pass, node or rewrite. |
 | `upstreamHost` _string_ | Specifies the host of the Upstream request. This is only valid if the pass_host is set to rewrite |
-| `discovery` _[Discovery](#discovery)_ | Discovery is used to configure service discovery for upstream. |
+| `discovery` _[Discovery](#discovery)_ | Deprecated: this is no longer support on standalone mode. Discovery is used to configure service discovery for upstream. |
 
 
 _Appears in:_
@@ -1391,12 +1391,12 @@ ApisixUpstreamSpec describes the specification of ApisixUpstream.
 | `scheme` _string_ | The scheme used to talk with the upstream. Now value can be http, grpc. |
 | `retries` _integer_ | How many times that the proxy (Apache APISIX) should do when errors occur (error, timeout or bad http status codes like 500, 502). |
 | `timeout` _[UpstreamTimeout](#upstreamtimeout)_ | Timeout settings for the read, send and connect to the upstream. |
-| `healthCheck` _[HealthCheck](#healthcheck)_ | The health check configurations for the upstream. |
+| `healthCheck` _[HealthCheck](#healthcheck)_ | Deprecated: this is no longer support on standalone mode. The health check configurations for the upstream. |
 | `tlsSecret` _[ApisixSecret](#apisixsecret)_ | Set the client certificate when connecting to TLS upstream. |
 | `subsets` _[ApisixUpstreamSubset](#apisixupstreamsubset) array_ | Subsets groups the service endpoints by their labels. Usually used to differentiate service versions. |
 | `passHost` _string_ | Configures the host when the request is forwarded to the upstream. Can be one of pass, node or rewrite. |
 | `upstreamHost` _string_ | Specifies the host of the Upstream request. This is only valid if the pass_host is set to rewrite |
-| `discovery` _[Discovery](#discovery)_ | Discovery is used to configure service discovery for upstream. |
+| `discovery` _[Discovery](#discovery)_ | Deprecated: this is no longer support on standalone mode. Discovery is used to configure service discovery for upstream. |
 | `portLevelSettings` _[PortLevelSettings](#portlevelsettings) array_ |  |
 
 
@@ -1560,12 +1560,12 @@ them if they are set on the port level.
 | `scheme` _string_ | The scheme used to talk with the upstream. Now value can be http, grpc. |
 | `retries` _integer_ | How many times that the proxy (Apache APISIX) should do when errors occur (error, timeout or bad http status codes like 500, 502). |
 | `timeout` _[UpstreamTimeout](#upstreamtimeout)_ | Timeout settings for the read, send and connect to the upstream. |
-| `healthCheck` _[HealthCheck](#healthcheck)_ | The health check configurations for the upstream. |
+| `healthCheck` _[HealthCheck](#healthcheck)_ | Deprecated: this is no longer support on standalone mode. The health check configurations for the upstream. |
 | `tlsSecret` _[ApisixSecret](#apisixsecret)_ | Set the client certificate when connecting to TLS upstream. |
 | `subsets` _[ApisixUpstreamSubset](#apisixupstreamsubset) array_ | Subsets groups the service endpoints by their labels. Usually used to differentiate service versions. |
 | `passHost` _string_ | Configures the host when the request is forwarded to the upstream. Can be one of pass, node or rewrite. |
 | `upstreamHost` _string_ | Specifies the host of the Upstream request. This is only valid if the pass_host is set to rewrite |
-| `discovery` _[Discovery](#discovery)_ | Discovery is used to configure service discovery for upstream. |
+| `discovery` _[Discovery](#discovery)_ | Deprecated: this is no longer support on standalone mode. Discovery is used to configure service discovery for upstream. |
 | `port` _integer_ | Port is a Kubernetes Service port, it should be already defined. |
 
 

internal/provider/adc/translator/apisixupstream.go

@@ -32,10 +32,8 @@ func (t *Translator) translateApisixUpstream(tctx *provider.TranslateContext, au
 		patchApisixUpstreamBasics,
 		translateApisixUpstreamScheme,
 		translateApisixUpstreamLoadBalancer,
-		translateApisixUpstreamHealthCheck,
 		translateApisixUpstreamRetriesAndTimeout,
 		translateApisixUpstreamPassHost,
-		translateApisixUpstreamDiscovery,
 	} {
 		if err = f(au, ups); err != nil {
 			return
@@ -98,83 +96,6 @@ func translateApisixUpstreamLoadBalancer(au *apiv2.ApisixUpstream, ups *adc.Upst
 	return nil
 }
 
-func translateApisixUpstreamHealthCheck(au *apiv2.ApisixUpstream, ups *adc.Upstream) error {
-	check := au.Spec.HealthCheck
-	if check == nil {
-		return nil
-	}
-
-	ups.Checks = new(adc.UpstreamHealthCheck)
-	ups.Checks.Active = translateApisixUpstreamHealthCheckActive(check.Active)
-
-	if check.Passive != nil {
-		ups.Checks.Passive = translateApisixUpstreamHealthCheckPassive(check.Passive)
-	}
-
-	return nil
-}
-
-func translateApisixUpstreamHealthCheckActive(config *apiv2.ActiveHealthCheck) *adc.UpstreamActiveHealthCheck {
-	var active adc.UpstreamActiveHealthCheck
-
-	active.Type = cmp.Or(config.Type, apiv2.HealthCheckHTTP)
-	active.Timeout = int(config.Timeout.Seconds())
-	active.Port = config.Port
-	active.Concurrency = config.Concurrency
-	active.Host = config.Host
-	active.HTTPPath = config.HTTPPath
-	active.HTTPRequestHeaders = config.RequestHeaders
-	active.HTTPSVerifyCert = config.StrictTLS != nil && *config.StrictTLS
-
-	if config.Healthy != nil {
-		active.Healthy = adc.UpstreamActiveHealthCheckHealthy{
-			UpstreamPassiveHealthCheckHealthy: adc.UpstreamPassiveHealthCheckHealthy{
-				HTTPStatuses: config.Healthy.HTTPCodes,
-				Successes:    config.Healthy.Successes,
-			},
-			Interval: int(config.Healthy.Interval.Seconds()),
-		}
-	}
-
-	if config.Unhealthy != nil {
-		active.Unhealthy = adc.UpstreamActiveHealthCheckUnhealthy{
-			UpstreamPassiveHealthCheckUnhealthy: adc.UpstreamPassiveHealthCheckUnhealthy{
-				HTTPStatuses: config.Unhealthy.HTTPCodes,
-				HTTPFailures: config.Unhealthy.HTTPFailures,
-				TCPFailures:  config.Unhealthy.TCPFailures,
-				Timeouts:     int(config.Timeout.Seconds()),
-			},
-			Interval: int(config.Unhealthy.Interval.Seconds()),
-		}
-	}
-
-	return &active
-}
-
-func translateApisixUpstreamHealthCheckPassive(config *apiv2.PassiveHealthCheck) *adc.UpstreamPassiveHealthCheck {
-	var passive adc.UpstreamPassiveHealthCheck
-
-	passive.Type = cmp.Or(config.Type, apiv2.HealthCheckHTTP)
-
-	if config.Healthy != nil {
-		passive.Healthy = adc.UpstreamPassiveHealthCheckHealthy{
-			HTTPStatuses: config.Healthy.HTTPCodes,
-			Successes:    config.Healthy.Successes,
-		}
-	}
-
-	if config.Unhealthy != nil {
-		passive.Unhealthy = adc.UpstreamPassiveHealthCheckUnhealthy{
-			HTTPStatuses: config.Unhealthy.HTTPCodes,
-			HTTPFailures: config.Unhealthy.HTTPFailures,
-			TCPFailures:  config.Unhealthy.TCPFailures,
-			Timeouts:     config.Unhealthy.Timeouts,
-		}
-	}
-
-	return &passive
-}
-
 func translateApisixUpstreamRetriesAndTimeout(au *apiv2.ApisixUpstream, ups *adc.Upstream) error {
 	retries := au.Spec.Retries
 	timeout := au.Spec.Timeout
@@ -248,28 +169,11 @@ func translateApisixUpstreamPassHost(au *apiv2.ApisixUpstream, ups *adc.Upstream
 	return nil
 }
 
-func translateApisixUpstreamDiscovery(au *apiv2.ApisixUpstream, ups *adc.Upstream) error {
-	if au.Spec.Discovery == nil {
-		return nil
-	}
-
-	ups.ServiceName = au.Spec.Discovery.ServiceName
-	ups.DiscoveryType = au.Spec.Discovery.Type
-	ups.DiscoveryArgs = au.Spec.Discovery.Args
-	ups.Nodes = nil
-
-	return nil
-}
-
 func composeExternalUpstreamName(au *apiv2.ApisixUpstream) string {
 	return au.GetGenerateName() + "_" + au.GetName()
 }
 
 func translateApisixUpstreamExternalNodes(tctx *provider.TranslateContext, au *apiv2.ApisixUpstream, ups *adc.Upstream) error {
-	if au.Spec.Discovery != nil {
-		ups.Nodes = nil
-		return nil
-	}
 	for _, node := range au.Spec.ExternalNodes {
 		switch node.Type {
 		case apiv2.ExternalTypeDomain:

test/e2e/apisix/route.go

@@ -496,123 +496,5 @@ spec:
 			Eventually(upstreamAddrs).Should(HaveKey(endpoint))
 			Eventually(upstreamAddrs).Should(HaveKey(clusterIP))
 		})
-
-		PIt("Test Upstream healthcheck", func() {
-			const apisixRouteSpec = `
-apiVersion: apisix.apache.org/v2
-kind: ApisixRoute
-metadata:
-  name: default
-spec:
-  ingressClassName: apisix
-  http:
-  - name: rule0
-    match:
-      paths:
-      - /*
-    upstreams:
-    - name: default-upstream
-`
-			const apisixUpstreamSpec0 = `
-apiVersion: apisix.apache.org/v2
-kind: ApisixUpstream
-metadata:
-  name: default-upstream
-spec:
-  ingressClassName: apisix
-  externalNodes:
-  - type: Domain
-    name: httpbin-service-e2e-test
-`
-			const apisixUpstreamSpec1 = `
-apiVersion: apisix.apache.org/v2
-kind: ApisixUpstream
-metadata:
-  name: default-upstream
-spec:
-  ingressClassName: apisix
-  externalNodes:
-  - type: Domain
-    name: httpbin-service-e2e-test
-  healthCheck:
-    active:
-      type: http
-      httpPath: %s
-      healthy:
-        httpCodes: [200]
-        interval: 1s
-      unhealthy:
-        httpFailures: 2
-        interval: 1s
-`
-			By("apply ApisixRoute")
-			applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "default"}, new(apiv2.ApisixRoute), apisixRouteSpec)
-
-			By("apply ApisixUpstream without healthCheck")
-			applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "default-upstream"}, new(apiv2.ApisixUpstream), apisixUpstreamSpec0)
-
-			By("verify ApisixRoute and ApisixUpstream works")
-			request := func() int {
-				return s.NewAPISIXClient().GET("/get").Expect().Raw().StatusCode
-			}
-			Eventually(request).WithTimeout(8 * time.Second).ProbeEvery(time.Second).Should(Equal(http.StatusOK))
-
-			By("apply ApisixUpstream with a healthCheck")
-			auSpec := fmt.Sprintf(apisixUpstreamSpec1, "/status/200")
-			applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "default-upstream"}, new(apiv2.ApisixUpstream), auSpec)
-
-			By("verify ApisixRoute and ApisixUpstream with a healthCheck works")
-			Eventually(request).WithTimeout(8 * time.Second).ProbeEvery(time.Second).Should(Equal(http.StatusOK))
-
-			By("apply ApisixUpstream with the healthCheck which is always failure")
-			auSpec = fmt.Sprintf(apisixUpstreamSpec1, "/status/502")
-			applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "default-upstream"}, new(apiv2.ApisixUpstream), auSpec)
-
-			// FIXME: can not pass yet
-			Skip("checks did not sync to the dataplane")
-			By("verify ApisixRoute and ApisixUpstream works")
-			Eventually(request).WithTimeout(8 * time.Second).ProbeEvery(time.Second).Should(Equal(http.StatusServiceUnavailable))
-		})
-
-		PIt("Test discovery", func() {
-			const apisixRouteSpec = `
-apiVersion: apisix.apache.org/v2
-kind: ApisixRoute
-metadata:
-  name: default
-spec:
-  ingressClassName: apisix
-  http:
-  - name: rule0
-    match:
-      paths:
-      - /*
-    upstreams:
-    - name: default-upstream
-`
-			const apisixUpstreamSpec0 = `
-apiVersion: apisix.apache.org/v2
-kind: ApisixUpstream
-metadata:
-  name: default-upstream
-spec:
-  ingressClassName: apisix
-  discovery:
-    type: dns
-    serviceName: httpbin-service-e2e-test.%s.svc.cluster.local
-`
-			By("apply ApisixRoute")
-			applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "default"}, new(apiv2.ApisixRoute), apisixRouteSpec)
-
-			By("apply ApisixUpstream with discovery")
-			applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "default-upstream"}, new(apiv2.ApisixUpstream), fmt.Sprintf(apisixUpstreamSpec0, s.Namespace()))
-
-			By("verify ApisixUpstream works")
-			time.Sleep(time.Hour)
-			request := func() int {
-				return s.NewAPISIXClient().GET("/get").Expect().Raw().StatusCode
-			}
-			Eventually(request).WithTimeout(8 * time.Second).ProbeEvery(time.Second).Should(Equal(http.StatusOK))
-		})
 	})
 })