Skip to content

Conversation

@ronething
Copy link
Contributor

@ronething ronething commented Sep 8, 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:

ref: #2540 (comment)

In the traffic-split plugin, use upstream_id to replace inline upstream.

  • dataplane config example:
{
  "upstreams_conf_version": 1757381144237,
  "services": [
    {
      "modifiedIndex": 1757381144237,
      "labels": {
        "k8s/kind": "HTTPRoute",
        "k8s/name": "httpbin",
        "k8s/namespace": "ingress-apisix-e2e-tests-default-382127000",
        "k8s/controller-name": "apisix.apache.org/apisix-ingress-controller/ingress-apisix-e2e-tests-default-382127000",
        "manager-by": "apisix-ingress-controller"
      },
      "hosts": [
        "httpbin.example"
      ],
      "plugins": {
        "traffic-split": {
          "rules": [
            {
              "weighted_upstreams": [
                {
                  "weight": 100
                },
                {
                  "weight": 0,
                  "upstream_id": "132f5f83"
                }
              ]
            }
          ]
        }
      },
      "upstream_id": "25898fae",
      "name": "ingress-apisix-e2e-tests-default-382127000_httpbin_0",
      "id": "25898fae"
    }
  ],
  "X-Digest": "4e8c5200539290ec21f0bbdd1b9ea0df99d8241b",
  "upstreams": [
    {
      "modifiedIndex": 1757381144237,
      "labels": {
        "managed-by": "apisix-ingress-controller"
      },
      "nodes": [
        {
          "weight": 100,
          "port": 80,
          "host": "10.244.0.53"
        }
      ],
      "scheme": "http",
      "type": "roundrobin",
      "name": "ingress-apisix-e2e-tests-default-382127000_httpbin_0",
      "id": "25898fae"
    },
    {
      "modifiedIndex": 1757381144237,
      "labels": {
        "managed-by": "apisix-ingress-controller",
        "__ADC_UPSTREAM_SERVICE_ID": "25898fae"
      },
      "nodes": [
        {
          "weight": 0,
          "port": 80,
          "host": "10.244.0.56"
        },
        {
          "weight": 0,
          "port": 80,
          "host": "10.244.0.57"
        }
      ],
      "scheme": "http",
      "type": "roundrobin",
      "name": "Service_ingress-apisix-e2e-tests-default-382127000_nginx_80",
      "id": "132f5f83"
    }
  ],
  "X-Last-Modified": 1757381144,
  "services_conf_version": 1757381144237
}

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

Signed-off-by: Ashing Zheng <[email protected]>
Signed-off-by: Ashing Zheng <[email protected]>
Signed-off-by: Ashing Zheng <[email protected]>
Signed-off-by: Ashing Zheng <[email protected]>
Signed-off-by: Ashing Zheng <[email protected]>
@ronething ronething marked this pull request as ready for review September 9, 2025 02:57
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 refactors the traffic-split plugin implementation to use upstream IDs instead of inline upstream objects, improving the dataplane configuration structure. The change ensures that upstreams are properly referenced by ID in traffic-split configurations while maintaining separate upstream definitions.

  • Replace inline upstream objects with upstream_id references in traffic-split plugin
  • Add proper upstream naming and ID generation for backend references
  • Update Service struct to include an Upstreams field for managing multiple upstream definitions

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
internal/adc/translator/httproute.go Adds upstream naming/ID generation and uses upstream_id in traffic-split configurations
internal/adc/translator/apisixroute.go Updates ApisixRoute translator to use upstream_id references and proper naming
api/v2/shared_types.go Adds utility functions for composing upstream names
api/adc/types.go Adds Upstreams field to Service struct
api/adc/zz_generated.deepcopy.go Generated deep copy methods for new Upstreams field
.github/workflows/apisix-e2e-test.yml Sets ADC_VERSION environment variable

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@ronething ronething changed the title fix: use upstream id instead of inline upstream fix: use upstream id instead of inline upstream in traffic-split plugin Sep 9, 2025
@ronething ronething requested a review from AlinsRan September 9, 2025 07:59
Signed-off-by: Ashing Zheng <[email protected]>
Copy link
Contributor

@bzp2010 bzp2010 left a comment

Choose a reason for hiding this comment

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

Are our tests based on the final results? Do they not need to be modified to verify the changes in the pull request?

@ronething
Copy link
Contributor Author

Are our tests based on the final results? Do they not need to be modified to verify the changes in the pull request?

The current test should be sufficient. If necessary, we can consider creating a new issue to track some more detailed assertions in the test.

@ronething ronething merged commit 7f6cff4 into master Sep 9, 2025
22 checks passed
@ronething ronething deleted the fix/inline_upstream branch September 9, 2025 15:40
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