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

Support disable service retry #211

Closed
wants to merge 2 commits into from
Closed

Support disable service retry #211

wants to merge 2 commits into from

Conversation

cnef
Copy link
Contributor

@cnef cnef commented Dec 17, 2018

I need to disable retry for service, but the proxy set retries to 0, it doesn't apply to kong and the DB doesn't update.

apiVersion: configuration.konghq.com/v1
kind: KongIngress
metadata:
  name: service1
proxy:
  path: /
  protocol: http
  retries: 0
route:
  preserve_host: true

I need to disable retry for service, but the proxy set retries to 0, it doesn't apply to kong and the DB doesn't update.
@@ -391,7 +391,7 @@ func (n *NGINXController) syncServices(ingressCfg *ingress.Configuration) (bool,
outOfSync = true
}

if kongIngress.Proxy.Retries > 0 &&
if kongIngress.Proxy.Retries >= 0 &&
Copy link
Member

Choose a reason for hiding this comment

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

The default value for kongIngress.Proxy.Retries will be 0, meaning that this branch will evaluate to true if there a KongIngress resource associated with an Ingress.
This will change the behavior as it will set the retries to 0.

The reason is, we can't differentiate between an explicit user setting it to 0 and an absent value.
This can be solved by using pointers instead of plain int.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, It's updated, please review again.

@hbagdi
Copy link
Member

hbagdi commented Dec 19, 2018

@zhxcai You will need to update the auto generated code using ./hack/update-codegen.sh.

@hbagdi
Copy link
Member

hbagdi commented Dec 20, 2018

@zhxcai I merged this in manually with b8bb7bd. It'll be available in the next release.

Thank you for your contribution!

@hbagdi hbagdi closed this Dec 20, 2018
@cnef cnef deleted the patch-1 branch December 26, 2018 06:35
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.

2 participants