Gateway api #1042
Gateway api #1042
Conversation
Codecov ReportPatch coverage has no change and project coverage change:
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## main #1042 +/- ##
==========================================
+ Coverage 81.02% 81.09% +0.06%
==========================================
Files 136 136
Lines 15405 15407 +2
==========================================
+ Hits 12482 12494 +12
+ Misses 2336 2327 -9
+ Partials 587 586 -1 ☔ View full report in Codecov by Sentry. |
|
|
||
| // DefaultSpec returns the default spec of GatewayController. | ||
| func (gc *GatewayController) DefaultSpec() interface{} { | ||
| return &Spec{} |
There was a problem hiding this comment.
IMO, ~/.kube/config could be the default value. And I think the GatewayController could support ClientSetInCluster mode[1].
There was a problem hiding this comment.
the default is in cluster mode(when both KubeConfig and MasterURL are empty) as it will be deployed in a k8s cluster at most of the time. And ClientSetInCluster mode is used, please check L161 in k8s.go.
| func (b *pipelineSpecBuilder) addBackend(addr string, weight int) { | ||
| if b.proxy == nil { | ||
| b.proxy = &httpproxy.Spec{} | ||
| b.proxy.Pools = append(b.proxy.Pools, &httpproxy.ServerPoolSpec{ServerMaxBodySize: -1}) |
There was a problem hiding this comment.
A side question, why not make the default value 0 for unlimited body?
There was a problem hiding this comment.
To support unlimited body, we have to enable the stream mode, but in this mode, some of the new features of v2.0 becomes unavailable. so the meaning of "set ServerMaxBodySize to zero" is a balance of different things, and we think using -1 for unlimited body is better for more cases.
pkg/object/gatewaycontroller/k8s.go
Outdated
| return true | ||
| } | ||
|
|
||
| if oldObj.(metav1.Object).GetResourceVersion() == newObj.(metav1.Object).GetResourceVersion() { |
There was a problem hiding this comment.
Just wonder why *apicorev1.Endpoints is not Object.
There was a problem hiding this comment.
apicorev1.Endpoints is Object, but this line is only for other objects, for endpoints, we will do additional checks below.
however, just found some of the conditions and return values of this function is wrong, will fix them.
| return false | ||
| } | ||
|
|
||
| func isSubsetsChanged(a, b apicorev1.EndpointSubset) bool { |
There was a problem hiding this comment.
Why reflect.DeepEqual can not cover this function.
There was a problem hiding this comment.
DeepEqual can be used here.
But for performance consideration, explicit comparsion is used.
pkg/object/gatewaycontroller/k8s.go
Outdated
|
|
||
| if minor < 19 { | ||
| // Ingress version v1 has been added after kubernetes 1.19 | ||
| panic(fmt.Errorf("kubernetes version [%v] is too low, GatewayController requires kubernetes v1.19+", info.GitVersion)) |
There was a problem hiding this comment.
In doc, the require of k8s is v1.18+, but here is v1.19...
No description provided.