Skip to content

Conversation

@AlinsRan
Copy link
Contributor

@AlinsRan AlinsRan commented Aug 1, 2025

Type of change:

  • Bugfix
  • New feature provided
  • Improve performance
  • Backport patches
  • Documentation
  • Refactor
  • Chore
  • CI/CD or Tests

What this PR does / why we need it:

Support running the ingress controller in Kuberntes 1.18.

  • Need to maintain a separate CRD without cel.
  • This cluster version does not support IngressClass/v1 and requires the use of IngressClass/v1beta1.

Pre-submission checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2025

conformance test report - apisix-standalone mode

apiVersion: gateway.networking.k8s.io/v1
date: "2025-08-04T02:40:12Z"
gatewayAPIChannel: standard
gatewayAPIVersion: v1.2.0
implementation:
  contact: null
  organization: APISIX
  project: apisix-ingress-controller
  url: https://github.com/apache/apisix-ingress-controller.git
  version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
    result: partial
    skippedTests:
    - HTTPRouteHTTPSListener
    statistics:
      Failed: 0
      Passed: 32
      Skipped: 1
  name: GATEWAY-HTTP
  summary: Core tests partially succeeded with 1 test skips.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2025

conformance test report - apisix mode

apiVersion: gateway.networking.k8s.io/v1
date: "2025-08-04T02:40:17Z"
gatewayAPIChannel: standard
gatewayAPIVersion: v1.2.0
implementation:
  contact: null
  organization: APISIX
  project: apisix-ingress-controller
  url: https://github.com/apache/apisix-ingress-controller.git
  version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
    result: partial
    skippedTests:
    - HTTPRouteHTTPSListener
    statistics:
      Failed: 0
      Passed: 32
      Skipped: 1
  name: GATEWAY-HTTP
  summary: Core tests partially succeeded with 1 test skips.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2025

conformance test report

apiVersion: gateway.networking.k8s.io/v1
date: "2025-08-04T02:44:06Z"
gatewayAPIChannel: standard
gatewayAPIVersion: v1.2.0
implementation:
  contact: null
  organization: APISIX
  project: apisix-ingress-controller
  url: https://github.com/apache/apisix-ingress-controller.git
  version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
    result: partial
    skippedTests:
    - HTTPRouteHTTPSListener
    statistics:
      Failed: 0
      Passed: 32
      Skipped: 1
  name: GATEWAY-HTTP
  summary: Core tests partially succeeded with 1 test skips.

@ronething ronething marked this pull request as ready for review August 1, 2025 08:24
@ronething ronething marked this pull request as draft August 1, 2025 08:24
@AlinsRan AlinsRan marked this pull request as ready for review August 4, 2025 00:50
@AlinsRan AlinsRan requested review from Copilot and ronething August 4, 2025 01:02
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for Kubernetes 1.18 by implementing compatibility with the networking.k8s.io/v1beta1 API for IngressClass resources. The feature automatically detects which IngressClass API version is available in the cluster and falls back to v1beta1 when v1 is not supported.

  • Introduces automatic API version detection for IngressClass resources (v1 vs v1beta1)
  • Adds a new IngressClassV1beta1Reconciler controller for handling v1beta1 IngressClass resources
  • Updates all controllers to work with both IngressClass API versions through dynamic version selection

Reviewed Changes

Copilot reviewed 30 out of 31 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
internal/controller/ingressclass_v1beta1_controller.go New controller implementation for IngressClass v1beta1 API
internal/controller/utils.go Adds utility functions for v1beta1 support and IngressClass conversion logic
pkg/utils/cluster.go Implements IngressClass v1beta1 to v1 conversion utility function
internal/types/k8s.go Extends type handling to support v1beta1 IngressClass objects
internal/provider/adc/adc.go Updates provider to handle both v1 and v1beta1 IngressClass types
internal/manager/run.go Registers v1beta1 schemes and updates readiness manager setup
internal/manager/controllers.go Implements conditional controller registration based on API availability
internal/controller/indexer/indexer.go Adds indexing support for v1beta1 IngressClass resources
internal/controller/gatewayproxy_controller.go Updates to handle different IngressClass API versions
test/e2e/framework/apisix_consts.go Adds INGRESS_VERSION environment variable for test configuration
test/e2e/crds/v2/*.go Updates test files to use dynamic IngressClass API versions
.github/workflows/e2e-test-k8s.yml New workflow for testing Kubernetes 1.18 compatibility
Comments suppressed due to low confidence (5)

internal/controller/ingressclass_v1beta1_controller.go:179

  • The parameter name 'IngressClassV1beta1' uses a capitalized acronym which is inconsistent with Go naming conventions. It should be 'ingressClassV1beta1' or simply 'ingressClass'.
func (r *IngressClassV1beta1Reconciler) processInfrastructure(tctx *provider.TranslateContext, IngressClassV1beta1 *v1beta1.IngressClass) error {

internal/controller/ingressclass_v1beta1_controller.go:79

  • The variable name 'IngressClassV1beta1' on lines 80, 86, 102, 105, 115 uses inconsistent casing with capitalized acronyms. Consider using 'ingressClass' for better readability.
func (r *IngressClassV1beta1Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {

internal/controller/ingressclass_v1beta1_controller.go:123

  • The function name 'listIngressClassV1beta1esForGatewayProxy' is awkward and unclear. Consider renaming to 'listIngressClassesForGatewayProxy' for better readability.
func (r *IngressClassV1beta1Reconciler) listIngressClassV1beta1esForGatewayProxy(ctx context.Context, obj client.Object) []reconcile.Request {

internal/controller/ingressclass_v1beta1_controller.go:154

  • The function name 'listIngressClassV1beta1esForSecret' is awkward and unclear. Consider renaming to 'listIngressClassesForSecret' for better readability.
func (r *IngressClassV1beta1Reconciler) listIngressClassV1beta1esForSecret(ctx context.Context, obj client.Object) []reconcile.Request {

internal/controller/utils.go:1230

  • The function name 'listIngressClassV1beta1RequestsForGatewayProxy' is too specific and awkward. Consider a more generic name that doesn't include the API version.
func listIngressClassV1beta1RequestsForGatewayProxy(

Comment on lines +104 to +115
time.Sleep(5 * time.Second)

By("create GatewayClass")
gatewayClassName := fmt.Sprintf("apisix-%d", time.Now().Unix())
err = s.CreateResourceFromStringWithNamespace(fmt.Sprintf(gatewayClass, gatewayClassName, s.GetControllerName()), "")
Expect(err).NotTo(HaveOccurred(), "creating GatewayClass")
time.Sleep(5 * time.Second)

By("create Gateway")
err = s.CreateResourceFromString(fmt.Sprintf(defaultGateway, gatewayClassName))
Expect(err).NotTo(HaveOccurred(), "creating Gateway")
time.Sleep(5 * time.Second)
Copy link

Copilot AI Aug 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using fixed time.Sleep in tests can lead to flaky tests. Consider using retry mechanisms or waiting for specific conditions instead of arbitrary sleep durations.

Suggested change
time.Sleep(5 * time.Second)
By("create GatewayClass")
gatewayClassName := fmt.Sprintf("apisix-%d", time.Now().Unix())
err = s.CreateResourceFromStringWithNamespace(fmt.Sprintf(gatewayClass, gatewayClassName, s.GetControllerName()), "")
Expect(err).NotTo(HaveOccurred(), "creating GatewayClass")
time.Sleep(5 * time.Second)
By("create Gateway")
err = s.CreateResourceFromString(fmt.Sprintf(defaultGateway, gatewayClassName))
Expect(err).NotTo(HaveOccurred(), "creating Gateway")
time.Sleep(5 * time.Second)
Eventually(func() error {
_, err := s.GetResourceYaml("GatewayProxy", "apisix-proxy-config")
return err
}, 30*time.Second, 1*time.Second).Should(Succeed(), "waiting for GatewayProxy to be available")
By("create GatewayClass")
gatewayClassName := fmt.Sprintf("apisix-%d", time.Now().Unix())
err = s.CreateResourceFromStringWithNamespace(fmt.Sprintf(gatewayClass, gatewayClassName, s.GetControllerName()), "")
Expect(err).NotTo(HaveOccurred(), "creating GatewayClass")
Eventually(func() error {
_, err := s.GetResourceYaml("GatewayClass", gatewayClassName)
return err
}, 30*time.Second, 1*time.Second).Should(Succeed(), "waiting for GatewayClass to be available")
By("create Gateway")
err = s.CreateResourceFromString(fmt.Sprintf(defaultGateway, gatewayClassName))
Expect(err).NotTo(HaveOccurred(), "creating Gateway")
Eventually(func() error {
_, err := s.GetResourceYaml("Gateway", "apisix")
return err
}, 30*time.Second, 1*time.Second).Should(Succeed(), "waiting for Gateway to be available")

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@ronething ronething left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

others lgtm.

@AlinsRan AlinsRan merged commit 8ae1f23 into release-v2-dev Aug 4, 2025
22 of 25 checks passed
@ronething ronething deleted the feat/k8s-1-18 branch August 4, 2025 07:09
Comment on lines +233 to +240
if service := gatewayProxy.Spec.Provider.ControlPlane.Service; service != nil {
if err := addProviderEndpointsToTranslateContext(tctx, r.Client, types.NamespacedName{
Namespace: gatewayProxy.GetNamespace(),
Name: service.Name,
}); err != nil {
return err
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is necessary to determine whether the call here is accurate. In subsequent functions, resolveServiceEndpoints(tctx, c, serviceNN, true, nil)

the param supportsEndpointSlice We set it to true by default, which theoretically would cause problems on k8s clusters that do not support EndpointSlice

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants