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 v2beta1 structure for ApisixRoute #572

Merged
merged 6 commits into from
Jul 7, 2021

Conversation

gxthrj
Copy link
Contributor

@gxthrj gxthrj commented Jul 5, 2021

Please answer these questions before submitting a pull request

@codecov-commenter
Copy link

codecov-commenter commented Jul 5, 2021

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 34.86%. Comparing base (b4a6889) to head (2760a50).
Report is 619 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #572      +/-   ##
==========================================
- Coverage   35.04%   34.86%   -0.18%     
==========================================
  Files          55       55              
  Lines        4557     4557              
==========================================
- Hits         1597     1589       -8     
- Misses       2742     2747       +5     
- Partials      218      221       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tokers
Copy link
Contributor

tokers commented Jul 6, 2021

@gxthrj Please settle the CI problems.

type ApisixRoute struct {
metav1.TypeMeta `json:",inline" yaml:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty" yaml:"metadata,omitempty"`
Spec *ApisixRouteSpec `json:"spec,omitempty" yaml:"spec,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

We may don't need the pointer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just copy the other fields from v2alpha1, Perhaps we can change these fields, do not use pointer since v2beta1.

type ApisixRoute struct {
metav1.TypeMeta `json:",inline" yaml:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty" yaml:"metadata,omitempty"`
Spec *ApisixRouteSpec `json:"spec,omitempty" yaml:"spec,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

We may don't need the pointer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ditto


// ApisixStatus is the status report for Apisix ingress Resources
type ApisixStatus struct {
Conditions *[]metav1.Condition `json:"conditions,omitempty" yaml:"conditions,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

I think a slice is enough, why we need a pointer which points to a slice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ditto

- name: v2alpha1
served: true
storage: false
Copy link
Contributor

Choose a reason for hiding this comment

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

We may still keep the v2alpha1 as the storage version, once the v2beta1 logic are completed, we can migrate from v2alpha1 to v2beta1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK

type: string
required:
- serviceName
- servicePort
Copy link
Contributor

Choose a reason for hiding this comment

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

EOL missing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -28,6 +29,8 @@ const (
ApisixRouteV1 = "apisix.apache.org/v1"
// ApisixRouteV2alpha1 represents the APisixRoute in apisix.apache.org/v2alpha1 group version
ApisixRouteV2alpha1 = "apisix.apache.org/v2alpha1"
// ApisixRouteV2beta1 represents the APisixRoute in apisix.apache.org/v2beta1 group version
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// ApisixRouteV2beta1 represents the APisixRoute in apisix.apache.org/v2beta1 group version
// ApisixRouteV2beta1 represents the ApisixRoute in apisix.apache.org/v2beta1 group version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@tao12345666333 tao12345666333 merged commit 534fab3 into apache:master Jul 7, 2021
fgksgf pushed a commit to fgksgf/apisix-ingress-controller that referenced this pull request Aug 14, 2021
* feat: add v2beta1 version for ApisixRoute

* fix: crd versions

* fix: storage version
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.

4 participants