Skip to content

Conversation

@ronething
Copy link
Contributor

@ronething ronething commented Jun 3, 2025

In order to support the testing of the apisix standalone mode and the subsequent maintenance of the open-source and enterprise versions, we have separated the code related to the framework.

Files starting with api7_ will not need to be pushed to the open-source repository in the future. This approach is also designed to minimize unnecessary work, as much of the code is difficult to abstract or requires rewriting.

Currently supported:

  1. Run open-source apisix-related tests with make e2e-test-stanalone (can be renamed if needed), without deploying components such as dashboard and dp-manager.

  2. make e2e-test still runs the original tests as normal.

Not supported:

  1. APISIX Consistency testing

  2. Some e2e tests still need to be adjusted.

For example, additionalGatewayGroups and apisixCli in Scaffold, and dashboardHTTPTunnel and dashboardHTTPSTunnel in the Framework need to be separated.

If further changes are made, this PR will continue to expand, which is not conducive to further review. Therefore, it is planned to advance it in multiple PR.

ronething added 3 commits June 3, 2025 17:52
Signed-off-by: ashing <[email protected]>
@ronething ronething changed the title feat: separate framework (WIP)feat: separate framework Jun 3, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jun 3, 2025

conformance test report

apiVersion: gateway.networking.k8s.io/v1
date: "2025-06-03T16:58:04Z"
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 changed the title (WIP)feat: separate framework test: support apisix standalone (part 1) Jun 3, 2025
Signed-off-by: ashing <[email protected]>
@ronething ronething requested review from AlinsRan and Copilot June 4, 2025 01:19
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 introduces support for running APISIX standalone mode by splitting the APISIX‑related code from the main framework and adjusting end-to-end tests accordingly. Key changes include:

  • Removal of dashboard tunnel functions from the legacy framework and addition of new utility functions (e.g. NewExpectResponse, ListPods, ExecCommandInPod, GetPodLogs, WaitPodsLog) to support standalone mode.
  • Adjustments in the Ingress deploy options and test scaffolding to integrate the new APISIX standalone tests.
  • Updates in dependency management (e.g. YAML package version changes) and CI workflow to run the new test target.

Reviewed Changes

Copilot reviewed 34 out of 34 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
test/e2e/framework/k8s.go Removed legacy dashboard tunnel functions and added new helper functions for pod handling and executing commands.
test/e2e/framework/ingress.go Removed deprecated fields from Ingress configuration.
test/e2e/framework/framework.go & api7_framework.go Removed old lifecycle functions; added new deployments and dashboard tunnel handling for APISIX standalone.
go.mod / internal/controller/config/config.go Updated YAML dependency versions and other dependency adjustments.
CI Workflow (.github/workflows/apisix-e2e-test.yml) Introduced a new test target for APISIX standalone mode.

Comment on lines +327 to +331
_ = exec.StreamWithContext(context.TODO(), remotecommand.StreamOptions{
Stdin: nil,
Stdout: &stdout,
Stderr: &stderr,
})
Copy link

Copilot AI Jun 4, 2025

Choose a reason for hiding this comment

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

The error returned by 'exec.StreamWithContext' is being ignored, which may hide failures during command execution. Consider capturing and checking the error instead of discarding it.

Suggested change
_ = exec.StreamWithContext(context.TODO(), remotecommand.StreamOptions{
Stdin: nil,
Stdout: &stdout,
Stderr: &stderr,
})
err = exec.StreamWithContext(context.TODO(), remotecommand.StreamOptions{
Stdin: nil,
Stdout: &stdout,
Stderr: &stderr,
})
f.GomegaT.Expect(err).ShouldNot(HaveOccurred(), "execute command in pod")

Copilot uses AI. Check for mistakes.
golang.org/x/net v0.28.0
gopkg.in/yaml.v2 v2.4.0
gorm.io/gorm v1.25.11
gopkg.in/yaml.v3 v3.0.1
Copy link

Copilot AI Jun 4, 2025

Choose a reason for hiding this comment

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

There appears to be an inconsistency in the YAML dependency versions between direct and indirect requirements. Please ensure consistent YAML version usage across the modules to avoid potential conflicts.

Suggested change
gopkg.in/yaml.v3 v3.0.1
// Removed gopkg.in/yaml.v3 to ensure consistent YAML library usage

Copilot uses AI. Check for mistakes.
time.Sleep(5 * time.Second)
}

func (f *Framework) newDashboardTunnel() error {
Copy link

Copilot AI Jun 4, 2025

Choose a reason for hiding this comment

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

The variables 'httpNodePort' and 'httpsNodePort' may remain zero if the expected port names ('http' and 'https') are missing from the service spec, which could lead to tunnel creation errors. Consider adding validation to ensure these port values are non-zero before proceeding.

Copilot uses AI. Check for mistakes.
@ronething ronething merged commit 27f6faf into release-v2-dev Jun 4, 2025
11 checks passed
@ronething ronething deleted the feat/support_apisix_standalone_test branch June 4, 2025 01:51
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