search

emissary-ingress / emissary

open source Kubernetes-native API gateway for microservices built on the Envoy Proxy
https://www.getambassador.io
Apache License 2.0
4.32k stars 684 forks source link

Golang boot sequence doesn't respect AMBASSADOR_NO_KUBEWATCH. #3228

Open eddy-curv opened 3 years ago

eddy-curv commented 3 years ago

Describe the bug Entrypoint of Ambassador versions prior to 1.10 didn't run watt when a when a value was provided to AMBASSADOR_NO_KUBEWATCH (no_kubewatch). However the new Golang boot sequence (default from 1.10) behaves differently - busyambassador always runs watt regardless of AMBASSADOR_NO_KUBEWATCH's value. This behaviour practically breaks configuration from the filesystem when running in K8s environment. The envoy configuration which was generated from ambassador/ambassador-config gets overridden with an empty configuration triggered by watt.

To Reproduce Steps to reproduce the behavior:

  1. Use Ambassador 1.10 with filesystem configuration (have at least one mapping to a K8s service in ambassador/ambassador-config).
  2. Deploy Ambassador on K8s environment (in our case GKE 1.16).
  3. Make sure that golang loader is used (i.e. don't use AMBASSADOR_LEGACY_MODE).
  4. Set AMBASSADOR_NO_KUBEWATCH=no_kubewatch.
  5. Make a request to the Ambassador service or open Ambassador diagnostics UI - there will be no mappings at all (apart from Ambassador's readiness and liveness if enabled).

Expected behavior When using the new golang loader I expect Ambassador not to override the envoy configuration generated from filesystem (mappings, TLS contexts etc...).

Versions:

Additional context The following events helped us to identify the issue:

  1. There are {aconf, econf, ir}-1. json and {aconf, econf, ir}. json files in snapshots/ directory.
  2. The snapshot.yaml file has an empty configuration.
  3. In ambassador logs we saw a request from watt which triggers the reconfiguration: 2021-02-14 11:01:20 diagd 1.11.1 [P20TThreadPoolExecutor-0_0] DEBUG: Update requested: watt, http://localhost:9696/snapshot
  4. cmd/entrypoint/env.go doesn't handle AMBASSADOR_NO_KUBEWATCH.
  5. Some screenshots are attached: https://ibb.co/cx8rKgJ https://ibb.co/w7XY4gh https://ibb.co/MSWxP2C
eddy-curv commented 3 years ago

I also asked in the Ambassador slack channel, and Noah Krause answered me:

Hey eddy, sorry for the late response. Recent versions of Ambassador have released with an update to how we are watching for resources from Kubernetes. This new entrypoint will always be watching for resources and does not respect the AMBASSADOR_NO_KUBEWATCH environment variable. Setting AMBASSADOR_LEGACY_MODE will revert back to the old resource watcher that does respect that environment variable and turn off the kubernetes watch hooks. Since AMBASSADOR_LEGACY_MODE is only changing how Ambassador watches for resources from Kubernetes, I do not think you should have any concern about setting that environment variable if you do not want Ambassador to watch for resources from Kubernetes.

So it seems that it isn't a bug but rather an expected behaviour of the new Goland loader. However you must update the documentation to reflect the new changed and which env vars are supported only in legacy mode.

kflynn commented 3 years ago

@eddy-curv It actually wasn't a design goal to drop it, so I'm OK calling this a bug. That said, AMBASSADOR_NO_KUBEWATCH was always a bit of a hack, which makes me a bit surprised to see the upvote count here!

If any of y'all want to comment on what you're using AMBASSADOR_NO_KUBEWATCH for, I'd love to know, to see if there's a less hackish way of covering what you need. Thanks!

eddy-curv commented 3 years ago

@kflynn, For about a year now we used Ambassador OSS with configuration from file system. This served us well for having a single source of truth for our local dev setup & CI (docker-compose based) and for our cloud setup (K8s - GKE). After an attempt to upgrade to Ambassador 1.10, we noticed the behaviour change introduced by the new golang loader. I referred to the Ambassador documentation (Configuration from the Filesystem) and expected that setting AMBASSADOR_NO_KUBEWATCH will respect configuration from filesystem. However, it didn't provide the expected results. If you can't or don't plan on making AMBASSADOR_NO_KUBEWATCH work with the new loader, at least update the documentation. We spent lot of time debugging and reading Ambassador's source code to understand what is going on. The perfect scenario will be to have all benefits of the new golang loader (e.g. support for Istio mTLS certificate rotation) while still being able to configure Ambassador from filesystem without using AMBASSADOR_LEGACY_MODE.

joshbranham commented 2 years ago

@eddy-curv It actually wasn't a design goal to drop it, so I'm OK calling this a bug. That said, AMBASSADOR_NO_KUBEWATCH was always a bit of a hack, which makes me a bit surprised to see the upvote count here!

If any of y'all want to comment on what you're using AMBASSADOR_NO_KUBEWATCH for, I'd love to know, to see if there's a less hackish way of covering what you need. Thanks!

We are also using this feature to run the project in local development environments in Docker. We are also hitting problems with this on emissary-ingress:2.2.2, where it seemingly is not reading this configuration even with AMBASSADOR_NO_KUBEWATCH.