You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2021/07/21 08:04:46 UTC

[GitHub] [apisix-ingress-controller] arabot777 opened a new issue #604: bug: the definition of timeout of passive unhealthy, ApisixUpsteam CRD resources and API definitions are inconsistent

arabot777 opened a new issue #604:
URL: https://github.com/apache/apisix-ingress-controller/issues/604


   ### Issue description
   Two questions.
   #### The first question is bug,  CRD resources and API object are different from timeount unit definitions.
   In CRD resources, passive healthy check timeout is set to **`string`**. In api, passive healthy check timeout is set to **`time.Duration`**. However In CRD resources, active healthy check timeout is set to **`number`** and in api is set to **`time.Duration`**.
   This leads to unable to declare  unhealthy timeout.
   https://github.com/apache/apisix-ingress-controller/blob/2d12c3fdd1369e9e263342d0def6473de5c0664f/samples/deploy/crd/v1beta1/ApisixUpstream.yaml#L341-L342
   https://github.com/apache/apisix-ingress-controller/blob/2d12c3fdd1369e9e263342d0def6473de5c0664f/pkg/kube/apisix/apis/config/v1/types.go#L218-L222
   
   
   #### The second question is suggestion.After testing, the timeout unit of the active health check is nanosecond instead of the second or milliseconds used frequently.This is still a little inconvenient for normal use.
   Few people think that the unit is nanosecond, or you can mark it in the document.
   If I want to define timeout for 5s, I need to do this.It is so big:
   
   > ```
   > apiVersion: apisix.apache.org/v1
   > kind: ApisixUpstream
   > metadata:
   >   name: httpbin
   > spec:
   >   healthCheck:
   >     active:
   >       type: http
   >       httpPath: /status/200
   >       timeout: 5000000000
   >       healthy:
   >         httpCodes: [200]
   >         interval: 1s
   >       unhealthy:
   >         httpFailures: 20
   >         interval: 1s
   > ```
   
   
   
   ### Environment
   
   apisix-ingress-controller version:
   Version: 0.6.0
   Git SHA: no-git-module
   Go Version: go1.13.8
   Building OS/Arch: linux/amd64
   Running OS/Arch: linux/amd64
   
   Kubernetes cluster version:
   Client Version: version.Info{Major:"1", Minor:"19", GitVersion:"v1.19.4", GitCommit:"d360454c9bcd1634cf4cc52d1867af5491dc9c5f", GitTreeState:"clean", BuildDate:"2020-11-12T01:09:16Z", GoVersion:"go1.15.4", Compiler:"gc", Platform:"darwin/amd64"}
   Server Version: version.Info{Major:"1", Minor:"20", GitVersion:"v1.20.2", GitCommit:"faecb196815e248d3ecfb03c680a4507229c2a56", GitTreeState:"clean", BuildDate:"2021-01-13T13:20:00Z", GoVersion:"go1.15.5", Compiler:"gc", Platform:"linux/amd64"}
   ### Minimal test code / Steps to reproduce the issue
   About the first question:
   1. If I am defined as int, it will report an error
   > ```
   > apiVersion: apisix.apache.org/v1
   > kind: ApisixUpstream
   > metadata:
   >   name: httpbin
   > spec:
   >   healthCheck:
   >     active:
   >       type: http
   >       httpPath: /status/200
   >       timeout: 5000000000
   >       healthy:
   >         httpCodes: [200]
   >         interval: 1s
   >       unhealthy:
   >         httpFailures: 20
   >         interval: 1s
   >     passive:
   >       type: http
   >       healthy:
   >         httpCodes: [200]
   >         successes: 20
   >       unhealthy:
   >         httpCodes: [502]
   >         httpFailures: 20
   >         tcpFailures: 20
   >         timeout: 5
   > ```
   
   the error logs :
   `The ApisixUpstream "httpbin" is invalid: spec.healthCheck.passive.unhealthy.timeout: Invalid value: "integer": spec.healthCheck.passive.unhealthy.timeout in body must be of type string: "integer"`
   
   2.If I am defined as string, it can't take effect
   > ```
   > apiVersion: apisix.apache.org/v1
   > kind: ApisixUpstream
   > metadata:
   >   name: httpbin
   > spec:
   >   healthCheck:
   >     active:
   >       type: http
   >       httpPath: /status/200
   >       timeout: 5000000000
   >       healthy:
   >         httpCodes: [200]
   >         interval: 1s
   >       unhealthy:
   >         httpFailures: 20
   >         interval: 1s
   >     passive:
   >       type: http
   >       healthy:
   >         httpCodes: [200]
   >         successes: 20
   >       unhealthy:
   >         httpCodes: [502]
   >         httpFailures: 20
   >         tcpFailures: 20
   >         timeout: '15'
   > ```
   ![image](https://user-images.githubusercontent.com/30978207/126453749-e1db3e17-1fe6-4477-91b1-22e65659823d.png)
   
   
   ### What's the actual result? (including assertion message & call stack if applicable)
   
   ### What's the expected result?
   Unified time unit definition, and inform the default unit


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] gxthrj commented on issue #604: bug: the definition of timeout of passive unhealthy, ApisixUpsteam CRD resources and API definitions are inconsistent

Posted by GitBox <gi...@apache.org>.
gxthrj commented on issue #604:
URL: https://github.com/apache/apisix-ingress-controller/issues/604#issuecomment-921778206


   It should be `timeouts` in `passive.unhealthy`, not `timeout`, which means the number of timeouts in proxied traffic to consider a target unhealthy.
   
   The logic is wrong in https://github.com/apache/apisix-ingress-controller/blob/master/pkg/kube/translation/apisix_upstream.go#L298
   
   And we should modify the ApisixUpstream CRD.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] tao12345666333 commented on issue #604: bug: the definition of timeout of passive unhealthy, ApisixUpsteam CRD resources and API definitions are inconsistent

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on issue #604:
URL: https://github.com/apache/apisix-ingress-controller/issues/604#issuecomment-892402371


   we can add check to admission. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] tokers commented on issue #604: bug: the definition of timeout of passive unhealthy, ApisixUpsteam CRD resources and API definitions are inconsistent

Posted by GitBox <gi...@apache.org>.
tokers commented on issue #604:
URL: https://github.com/apache/apisix-ingress-controller/issues/604#issuecomment-884111062


   > In CRD resources, passive healthy check timeout is set to string. In api, passive healthy check timeout is set to time.Duration. However In CRD resources, active healthy check timeout is set to number and in api is set to time.Duration.
   This leads to unable to declare unhealthy timeout.
   
   PR's welcome to fix this.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] tokers commented on issue #604: bug: the definition of timeout of passive unhealthy, ApisixUpsteam CRD resources and API definitions are inconsistent

Posted by GitBox <gi...@apache.org>.
tokers commented on issue #604:
URL: https://github.com/apache/apisix-ingress-controller/issues/604#issuecomment-897582852


   > i will submit pr. What do I need to pay attention to
   
   Do you mean the admission server? We have another friend who is devoting to this. Maybe you can wait for a while and see whether the PR can solve your problems.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] tao12345666333 commented on issue #604: bug: the definition of timeout of passive unhealthy, ApisixUpsteam CRD resources and API definitions are inconsistent

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on issue #604:
URL: https://github.com/apache/apisix-ingress-controller/issues/604#issuecomment-926718526


   #687 has been merged. Should we close this one? @gxthrj 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] gxthrj commented on issue #604: bug: the definition of timeout of passive unhealthy, ApisixUpsteam CRD resources and API definitions are inconsistent

Posted by GitBox <gi...@apache.org>.
gxthrj commented on issue #604:
URL: https://github.com/apache/apisix-ingress-controller/issues/604#issuecomment-927213265


   Yes, close this issue, feel free to reopen.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] tokers commented on issue #604: bug: the definition of timeout of passive unhealthy, ApisixUpsteam CRD resources and API definitions are inconsistent

Posted by GitBox <gi...@apache.org>.
tokers commented on issue #604:
URL: https://github.com/apache/apisix-ingress-controller/issues/604#issuecomment-884113769


   @arabot777 For the second one, we may create an our own time duration format which implements the `json.Unmarshaler` interface, so that we can pass some string values like `5s`, `10h` to it.
   
   See pkg/types/timeduration.go for details.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] arabot777 commented on issue #604: bug: the definition of timeout of passive unhealthy, ApisixUpsteam CRD resources and API definitions are inconsistent

Posted by GitBox <gi...@apache.org>.
arabot777 commented on issue #604:
URL: https://github.com/apache/apisix-ingress-controller/issues/604#issuecomment-899176629


   Understood. We hope the problem can be solved quickly and want to support plugins` client-control`


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] gxthrj closed issue #604: bug: the definition of timeout of passive unhealthy, ApisixUpsteam CRD resources and API definitions are inconsistent

Posted by GitBox <gi...@apache.org>.
gxthrj closed issue #604:
URL: https://github.com/apache/apisix-ingress-controller/issues/604


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-ingress-controller] arabot777 commented on issue #604: bug: the definition of timeout of passive unhealthy, ApisixUpsteam CRD resources and API definitions are inconsistent

Posted by GitBox <gi...@apache.org>.
arabot777 commented on issue #604:
URL: https://github.com/apache/apisix-ingress-controller/issues/604#issuecomment-897383509


   i will submit pr. What do I need to pay attention to


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org