carvel-dev / kapp-controller

Continuous delivery and package management for Kubernetes.
https://carvel.dev/kapp-controller
Apache License 2.0
260 stars 99 forks source link

kapp-controller fails to come up when cluster is configured with proxy #391

Closed ssamrit-vmw closed 2 years ago

ssamrit-vmw commented 2 years ago

Describe the problem/challenge you have When TMC attached cluster is configured with proxy and kapp-controller-config is created with the http and https proxy URL, kapp-controller fails to communicate with k8s API server.

{"level":"error","ts":1631181446.8004937,"logger":"controller-runtime.manager","msg":"Failed to get API Group-Resources","error":"Get \"https://10.96.0.1:443/api?timeout=32s\": net/http: request canceled while waiting for connection (Client.Timeout exceeded while awaiting headers)","stacktrace":"github.com/go-logr/zapr.(*zapLogger).Error\n\tgithub.com/go-logr/zapr@v0.2.0/zapr.go:132\nsigs.k8s.io/controller-runtime/pkg/manager.New\n\tsigs.k8s.io/controller-runtime@v0.7.0/pkg/manager/manager.go:317\ngithub.com/vmware-tanzu/carvel-kapp-controller/cmd/controller.Run\n\tgithub.com/vmware-tanzu/carvel-kapp-controller/cmd/controller/run.go:61\nmain.main\n\tcommand-line-arguments/main.go:41\nruntime.main\n\truntime/proc.go:225"}

Describe the solution you'd like After adding $(KUBERNETES_SERVICE_HOST) which is 10.96.0.1 in the noProxy list of kapp-controller-config, the kapp-controller started working. We would like the kapp-controller to populate the required noProxy URLs when it is configured with proxy.

As per the discussion with @cppforlife, kapp-controller can add config that enables auto-population of noProxy list.

Anything else you would like to add: kapp-controller version: https://github.com/vmware-tanzu/carvel-kapp-controller/releases/tag/v0.23.0


Vote on this request

This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.

👍 "I would like to see this addressed as soon as possible" 👎 "There are other more important things to focus on right now"

We are also happy to receive and review Pull Requests if you want to help working on this issue.

danielhelfand commented 2 years ago

Thanks for opening this, @ssamrit-vmw.

We would like the kapp-controller to populate the required noProxy URLs when it is configured with proxy.

Is there anything else in your mind that should be auto-populated in this feature besides KUBERNETES_SERVICE_HOST?

kapp-controller can add config that enables auto-population of noProxy list

What would be the ideal user experience with this feature? Should kapp-controller just have an opinionated way of populating this when a proxy is configured?

Should a user need to configure a property in kapp-controller's config to have this auto-population occur?

ssamrit-vmw commented 2 years ago

Is there anything else in your mind that should be auto-populated in this feature besides KUBERNETES_SERVICE_HOST?

Adding just KUBERNETES_SERVICE_HOST worked for us, but not sure if we should add localhost,127.0.0.1,kubernetes.default.svc,.svc as well as mentioned in https://kb.vmware.com/s/article/82129

What would be the ideal user experience with this feature? Should kapp-controller just have an opinionated way of populating this when a proxy is configured? Should a user need to configure a property in kapp-controller's config to have this auto-population occur?

I think anything should work for us as long as it honours noProxy list (if provided) along with default list. I think @cppforlife suggested introducing some property in configuration.

I have cc'ed you in the slack thread.

cppforlife commented 2 years ago

Should a user need to configure a property in kapp-controller's config to have this auto-population occur?

yeah, i dont think we should magically modify proxy config (that's provided) without user opting into this behaviour. im thinking just an additional field could enable this.

danielhelfand commented 2 years ago

Adding just KUBERNETES_SERVICE_HOST worked for us, but not sure if we should add localhost,127.0.0.1,kubernetes.default.svc,.svc as well as mentioned in https://kb.vmware.com/s/article/82129

We can always start small and simply go with KUBERNETES_SERVICE_HOST and then add more later on.

im thinking just an additional field could enable this.

I think adding an additional config property to enable this makes sense.

cppforlife commented 2 years ago

We can always start small and simply go with KUBERNETES_SERVICE_HOST and then add more later on.

i meant even KUBERNETES_SERVICE_HOST this is too much for auto inclusion imho. somebody should opt in because otherwise they wouldnt know that this stuff is being added.

danielhelfand commented 2 years ago

Reading more from an internal slack thread, I saw this suggestion:

Original question:

cant kapp add it automatically when the proxy is enabled? What happens when the ip address changes for some reason?

we can probably add a config that enables that behaviour. can you file an issue for this specifically

I guess what is unclear here is what is mean by config. Can you elaborate a bit more on this?

cppforlife commented 2 years ago

example:

apiVersion: v1
kind: Secret
metadata:
  name: kapp-controller-config
  namespace: kapp-controller
stringData:
  httpProxy: proxy-svc.proxy-server.svc.cluster.local:80
  httpsProxy: ""
  noProxy: "github.com,docker.io"
  noProxyAddKubernetesAPIServer: "true" # <-------- add this field
  dangerousSkipTLSVerify: "private-registry.com,insecure-registry.com"

when the field is there then kc can do its magic. alternatively have something like this:

apiVersion: v1
kind: Secret
metadata:
  name: kapp-controller-config
  namespace: kapp-controller
stringData:
  httpProxy: proxy-svc.proxy-server.svc.cluster.local:80
  httpsProxy: ""
  noProxy: "github.com,docker.io,$(KAPPCTRL_KUBERNETES_API)" # <--- replace with magic stuff
  dangerousSkipTLSVerify: "private-registry.com,insecure-registry.com"
ssamrit-vmw commented 2 years ago

@danielhelfand any update on this? When can we expect this to get addressed?

danielhelfand commented 2 years ago

@danielhelfand any update on this? When can we expect this to get addressed?

Hey @ssamrit-vmw,

Sorry for delayed response here. We are looking to prioritize and address this issue. Hopefully can provide further details next week.

danielhelfand commented 2 years ago

For option 2 UX below , could we allow passing in environment vars that kapp-controller could interpret and pass on to noProxy? This way KUBERNETES_SERVICE_HOST could be read in and used. Not sure if this pattern could be applied more widely for these configuration options.

apiVersion: v1
kind: Secret
metadata:
  name: kapp-controller-config
  namespace: kapp-controller
stringData:
  httpProxy: proxy-svc.proxy-server.svc.cluster.local:80
  httpsProxy: ""
  noProxy: "github.com,docker.io,$(KAPPCTRL_KUBERNETES_API)" # <--- replace with magic stuff
  dangerousSkipTLSVerify: "private-registry.com,insecure-registry.com"
cppforlife commented 2 years ago

could we allow passing in environment vars that kapp-controller could interpret and pass on to noProxy?

imho that way too "open". im also not coming up with any good examples where this would be useful. for k8s api it's a special case since it's only readily available inside the container, but for other configuration it should be possible to inject from outside (aka modify config secret directly).

im also concerned using shell syntax $(), $..., ${}, since that somehow might get picked up and interpolated by non-intended tooling (eg somebody pasted this into their bash script, and bash did a replacement with empty string)

lubronzhan commented 2 years ago

Hi @ssamrit-vmw did you added the serviceCIDR of the cluster to the NO_PROXY before you added KUBERNETES_SERVICE_HOST?

danniel1205 commented 2 years ago

@ssamrit-vmw , is your cluster provisioned by using vmw tanzu ? If so, we do have no proxy list populated for kapp-controller. https://github.com/vmware-tanzu/tanzu-framework/blob/96ab13f49bf9e7b8fe417eeaaee1a1a218c8e47f/pkg/v1/providers/ytt/lib/helpers.star#L283

ssamrit-vmw commented 2 years ago

Hi @ssamrit-vmw did you added the serviceCIDR of the cluster to the NO_PROXY before you added KUBERNETES_SERVICE_HOST?

@lubronzhan No, I have not added that. We were testing on kind cluster where proxy is configured via TMC.

ssamrit-vmw commented 2 years ago

@ssamrit-vmw , is your cluster provisioned by using vmw tanzu ? If so, we do have no proxy list populated for kapp-controller. https://github.com/vmware-tanzu/tanzu-framework/blob/96ab13f49bf9e7b8fe417eeaaee1a1a218c8e47f/pkg/v1/providers/ytt/lib/helpers.star#L283

@danniel1205 No, it was kind cluster created on local machine and then it was attached to TMC. We are working on adding carvel packaging feature in TMC.