Skip to content

Commit

Permalink
Correctly handle watchNamespace flag parsing errors (#493)
Browse files Browse the repository at this point in the history
Previously, we shipped a feature (d3b23cd) where multi-namespace watch
mode was supported. It turns out that the logic was completely flipped.
We need to assign namespace when the error is NIL and not vice versa. If
the error is NOT NIL, we need to block the controller initiation and
throw an error.

How did we catch this? Nothing special, just reviewing the generated
code and asking who on earth wrote that (me).

This indeed urges us to do proper `CARM`/`multi-namespace`/`cross-region`
testing and write some e2e tests that we can catch these issue very early on ...
When the `code-generator` PRs are drafted.

The current infra doesn't allow us to that, so we'll hack something
around next week.

Signed-off-by: Amine Hilaly <[email protected]>

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
  • Loading branch information
a-hilaly authored Jan 20, 2024
1 parent 6200884 commit 3667d16
Showing 1 changed file with 10 additions and 3 deletions.
13 changes: 10 additions & 3 deletions templates/cmd/controller/main.go.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,16 @@ func main() {
watchNamespaces := make(map[string]ctrlrtcache.Config, 0)
namespaces, err := ackCfg.GetWatchNamespaces()
if err != nil {
for _, namespace := range namespaces {
watchNamespaces[namespace] = ctrlrtcache.Config{}
}
setupLog.Error(
err, "Unable to parse watch namespaces.",
"aws.service", ackCfg.WatchNamespace,
)
os.Exit(1)
}
{{/* If namespaces is an empty slice, then we watch all namespaces */}}
{{/* If namespaces is a slice with multiple elements, then we watch only those namespaces */}}
for _, namespace := range namespaces {
watchNamespaces[namespace] = ctrlrtcache.Config{}
}
mgr, err := ctrlrt.NewManager(ctrlrt.GetConfigOrDie(), ctrlrt.Options{
Scheme: scheme,
Expand Down

0 comments on commit 3667d16

Please sign in to comment.