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

e2e-test: support ApisixRoute v2 #1103

Merged
merged 16 commits into from
Jul 1, 2022

Conversation

AlinsRan
Copy link
Contributor

@AlinsRan AlinsRan commented Jun 27, 2022

Type of change:

  • Bugfix
  • New feature provided
  • Improve performance
  • Backport patches

What this PR does / why we need it:

fix: #1108

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

@codecov-commenter
Copy link

codecov-commenter commented Jun 27, 2022

Codecov Report

Merging #1103 (40730de) into master (0e1f8d4) will decrease coverage by 0.04%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #1103      +/-   ##
==========================================
- Coverage   30.38%   30.34%   -0.05%     
==========================================
  Files          81       81              
  Lines        9390     9403      +13     
==========================================
  Hits         2853     2853              
- Misses       6222     6235      +13     
  Partials      315      315              
Impacted Files Coverage Δ
pkg/ingress/status.go 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b1dc75e...40730de. Read the comment docs.

@AlinsRan AlinsRan marked this pull request as ready for review June 30, 2022 03:53
test/e2e/scaffold/scaffold.go Outdated Show resolved Hide resolved
test/e2e/scaffold/scaffold.go Outdated Show resolved Hide resolved
@tao12345666333 tao12345666333 added this to the 1.5.0 milestone Jun 30, 2022
Comment on lines +138 to +146
suites(scaffold.NewScaffold(&scaffold.Options{
Name: "endpoints-port",
Kubeconfig: scaffold.GetKubeconfig(),
APISIXConfigPath: "testdata/apisix-gw-config.yaml",
IngressAPISIXReplicas: 1,
HTTPBinServicePort: 8080,
ApisixResourceVersion: scaffold.ApisixResourceVersion().V2beta3,
}))
})
Copy link
Member

Choose a reason for hiding this comment

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

how ablout using suites(scaffold.NewDefaultScaffold()) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

how ablout using suites(scaffold.NewDefaultScaffold()) ?

Separate configuration item HTTPBinServicePort:8080 are used here.

Comment on lines +147 to +155
ginkgo.Describe("suite-endpoints: scaffold v2", func() {
suites(scaffold.NewScaffold(&scaffold.Options{
Name: "endpoints-port",
Kubeconfig: scaffold.GetKubeconfig(),
APISIXConfigPath: "testdata/apisix-gw-config.yaml",
IngressAPISIXReplicas: 1,
HTTPBinServicePort: 8080,
ApisixResourceVersion: scaffold.ApisixResourceVersion().V2,
}))
Copy link
Member

Choose a reason for hiding this comment

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

ditto

Copy link
Contributor Author

@AlinsRan AlinsRan Jul 1, 2022

Choose a reason for hiding this comment

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

Separate configuration item HTTPBinServicePort:8080 are used here.

Comment on lines +221 to +229
suites(scaffold.NewScaffold(&scaffold.Options{
Name: "sync",
Kubeconfig: scaffold.GetKubeconfig(),
APISIXConfigPath: "testdata/apisix-gw-config.yaml",
IngressAPISIXReplicas: 1,
HTTPBinServicePort: 80,
ApisixResourceVersion: scaffold.ApisixResourceVersion().V2beta3,
ApisixResourceSyncInterval: "60s",
}))
Copy link
Member

Choose a reason for hiding this comment

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

ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Separate configuration item ApisixResourceSyncInterval: "60s" are used here.

Comment on lines +232 to +240
suites(scaffold.NewScaffold(&scaffold.Options{
Name: "sync",
Kubeconfig: scaffold.GetKubeconfig(),
APISIXConfigPath: "testdata/apisix-gw-config.yaml",
IngressAPISIXReplicas: 1,
HTTPBinServicePort: 80,
ApisixResourceVersion: scaffold.ApisixResourceVersion().V2,
ApisixResourceSyncInterval: "60s",
}))
Copy link
Member

Choose a reason for hiding this comment

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

ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Separate configuration item ApisixResourceSyncInterval: "60s" are used here.

Comment on lines +66 to +86
ginkgo.Describe("suite-ingress: scaffold v2beta3", func() {
suites(scaffold.NewScaffold(&scaffold.Options{
Name: "webhook",
Kubeconfig: scaffold.GetKubeconfig(),
APISIXConfigPath: "testdata/apisix-gw-config.yaml",
IngressAPISIXReplicas: 1,
HTTPBinServicePort: 80,
ApisixResourceVersion: scaffold.ApisixResourceVersion().V2beta3,
EnableWebhooks: false,
}))
})
ginkgo.Describe("suite-ingress: scaffold v2", func() {
suites(scaffold.NewScaffold(&scaffold.Options{
Name: "webhook",
Kubeconfig: scaffold.GetKubeconfig(),
APISIXConfigPath: "testdata/apisix-gw-config.yaml",
IngressAPISIXReplicas: 1,
HTTPBinServicePort: 80,
ApisixResourceVersion: scaffold.ApisixResourceVersion().V2,
EnableWebhooks: false,
}))
Copy link
Member

Choose a reason for hiding this comment

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

ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is to identify enablewebhooks.

return s.CreateResourceFromStringWithNamespace(apc, namespace)
}
errString := fmt.Sprint("the resource ", kindValue, " does not support")
return errors.New(errString)
Copy link
Member

Choose a reason for hiding this comment

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

fmt.Errorf

)

func (s *Scaffold) replaceApiVersion(yml, ver string) string {
return versionRegex.ReplaceAllString(yml, "apiVersion: "+ver+"\n")
}

func (s *Scaffold) getKindValue(yml string) string {
kind := strings.Replace(kindRegex.FindString(yml), "\n", "", -1)
Copy link
Member

Choose a reason for hiding this comment

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

Why not use capture groups?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Didn't notice.

Copy link
Member

Choose a reason for hiding this comment

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

How about a fix in a follow-up PR? @AlinsRan

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about a fix in a follow-up PR? @AlinsRan

Sure.

Copy link
Member

@tao12345666333 tao12345666333 left a comment

Choose a reason for hiding this comment

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

This PR is very big, basically LGTM

Let's merge first (I like the split of CI jobs in this PR, it can greatly reduce the time consuming of CI)

@tao12345666333 tao12345666333 merged commit 4aa2ca5 into apache:master Jul 1, 2022
@tao12345666333
Copy link
Member

let's move forward. thanks

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

Successfully merging this pull request may close these issues.

bug: ApisixRoute v2 version resource is missing status information.
4 participants