Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add experimental conformance support #1780

Merged
merged 33 commits into from
Sep 12, 2023

Conversation

Xunzhuo
Copy link
Member

@Xunzhuo Xunzhuo commented Aug 9, 2023

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

  • add experimental conformance support
  • generate conformance report

@codecov
Copy link

codecov bot commented Aug 9, 2023

Codecov Report

Merging #1780 (3d1ebac) into main (5cb8697) will increase coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1780      +/-   ##
==========================================
+ Coverage   65.32%   65.35%   +0.02%     
==========================================
  Files          86       86              
  Lines       12529    12529              
==========================================
+ Hits         8185     8188       +3     
+ Misses       3826     3824       -2     
+ Partials      518      517       -1     

see 2 files with indirect coverage changes

@arkodg
Copy link
Contributor

arkodg commented Aug 9, 2023

thanks for working on this @Xunzhuo !
do you think we should manually run this, run this in CI on every merge and copy into the latest release notes, run this before every semvar release ?
and how often should we publish this data upstream in the gateway api repo

@Xunzhuo Xunzhuo force-pushed the add-exp-conformance branch from da6ca58 to d8b1e37 Compare August 10, 2023 03:15
@Xunzhuo
Copy link
Member Author

Xunzhuo commented Aug 10, 2023

  apiVersion: gateway.networking.k8s.io/v1alpha1
  date: "2023-08-10T03:35:39Z"
  gatewayAPIVersion: v0.8.0-rc1
  implementation:
    contact:
    - https://github.com/envoyproxy/gateway/blob/main/GOVERNANCE.md
    organization: envoyproxy
    project: envoy-gateway
    url: https://github.com/envoyproxy/gateway
    version: latest
  kind: ConformanceReport
  profiles:
  - core:
      result: success
      statistics:
        Failed: 0
        Passed: 29
        Skipped: 0
      summary: ""
    extended:
      result: success
      statistics:
        Failed: 0
        Passed: 9
        Skipped: 0
      summary: ""
      supportedFeatures:
      - HTTPRouteSchemeRedirect
      - HTTPRoutePathRewrite
      - HTTPRouteQueryParamMatching
      - HTTPResponseHeaderModification
      - HTTPRoutePortRedirect
      - HTTPRoutePathRedirect
      - HTTPRouteHostRewrite
      - HTTPRouteRequestMirror
      - HTTPRouteMethodMatching
    name: HTTP
  - core:
      result: success
      statistics:
        Failed: 0
        Passed: 11
        Skipped: 0
      summary: ""
    name: TLS

@Xunzhuo
Copy link
Member Author

Xunzhuo commented Aug 10, 2023

thanks for working on this @Xunzhuo ! do you think we should manually run this, run this in CI on every merge and copy into the latest release notes, run this before every semvar release ?

I think experimental conformance can run in main CI, and having conformance report in every commit.

and how often should we publish this data upstream in the gateway api repo

I think each time we bump gwapi version ?

@arkodg
Copy link
Contributor

arkodg commented Aug 10, 2023

should we only run & publish this every time we import a new gateway api version ?
so we can add a new GHA that checks if charts/gateway-helm/crds/gatewayapi-crds.yaml path has changed and if so it runs this specific make target as well as publishes the Conformance Profile as a GH comment on the PR 😎 , which we can then copy over to upstream

@Xunzhuo
Copy link
Member Author

Xunzhuo commented Aug 11, 2023

should we only run & publish this every time we import a new gateway api version ? so we can add a new GHA that checks if charts/gateway-helm/crds/gatewayapi-crds.yaml path has changed and if so it runs this specific make target as well as publishes the Conformance Profile as a GH comment on the PR 😎 , which we can then copy over to upstream

Sounds good : )

@Xunzhuo Xunzhuo force-pushed the add-exp-conformance branch from b104778 to 4888ace Compare August 14, 2023 03:04
@Xunzhuo Xunzhuo force-pushed the add-exp-conformance branch from 4888ace to ab21a9b Compare August 25, 2023 03:23
@Xunzhuo Xunzhuo force-pushed the add-exp-conformance branch from ab21a9b to 2204b2c Compare September 7, 2023 03:02
@Xunzhuo
Copy link
Member Author

Xunzhuo commented Sep 8, 2023

I tested and made changes to gwapi.yaml, https://github.com/envoyproxy/gateway/actions/runs/6117087341?pr=1780, it worked normally.

@Xunzhuo Xunzhuo marked this pull request as ready for review September 8, 2023 02:57
@Xunzhuo Xunzhuo requested a review from a team as a code owner September 8, 2023 02:57
@Xunzhuo Xunzhuo requested review from a team, kflynn, LanceEa and chauhanshubham and removed request for a team September 8, 2023 03:06

jobs:
experimental-conformance-test:
permissions: write-all
Copy link
Contributor

Choose a reason for hiding this comment

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

any way to descope this a bit ?

Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
@Xunzhuo Xunzhuo force-pushed the add-exp-conformance branch from 29afd04 to df89a88 Compare September 12, 2023 02:57
Signed-off-by: bitliu <bitliu@tencent.com>
@Xunzhuo Xunzhuo requested a review from arkodg September 12, 2023 03:16
Signed-off-by: bitliu <bitliu@tencent.com>
Signed-off-by: bitliu <bitliu@tencent.com>
Debug: *flags.ShowDebug,
CleanupBaseResources: *flags.CleanupBaseResources,
SkipTests: skipTests,
SupportedFeatures: sets.Set[suite.SupportedFeature]{}.Insert(suite.HTTPRouteExtendedFeatures.UnsortedList()...),
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

@arkodg arkodg left a comment

Choose a reason for hiding this comment

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

thanks for adding another great tool to EG @Xunzhuo !
minor comment which can be done as a follow up

@Alice-Lilith Alice-Lilith merged commit 958ca58 into envoyproxy:main Sep 12, 2023
21 checks passed
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