Make per-stack hostnames configurable #693
Conversation
| @@ -570,7 +570,7 @@ func TestGenerateIngressSegmentWithSyncAnnotations(t *testing.T) { | |||
| ingressAnnotationsToSync: tc.ingresssAnnotationsToSync, | |||
| syncAnnotationsInIngress: tc.syncAnnotationsInIngress, | |||
| } | |||
| res, _ := c.GenerateIngressSegment() | |||
| res, _ := c.GenerateIngressSegment(true) | |||
There was a problem hiding this comment.
Can we duplicate this block of tests for GenerateIngressSegment(false) or would the result just be empty?
There was a problem hiding this comment.
I think the result would just be empty, because ingresses wouldn't be generated.
|
The default value of the flag e2e is also currently failing as the flag doesn't completely "disable" the per-stack ingresses. Looking into it now. |
|
Will test e2e tests by setting default to Update: This works, will disable the hostnames again and try to get e2es to pass. |
This reverts commit 19a2a55.
| @@ -70,6 +71,7 @@ func main() { | |||
| kingpin.Flag("enable-configmap-support", "Enable support for ConfigMaps on StackSets.").Default("false").BoolVar(&config.ConfigMapSupportEnabled) | |||
| kingpin.Flag("enable-secret-support", "Enable support for Secrets on StackSets.").Default("false").BoolVar(&config.SecretSupportEnabled) | |||
| kingpin.Flag("enable-pcs-support", "Enable support for PlatformCredentialsSet on StackSets.").Default("false").BoolVar(&config.PCSSupportEnabled) | |||
| kingpin.Flag("enable-per-stack-hostname", "Enable support for per-stack hostnames.").Default("false").BoolVar(&config.PerStackHostnameEnabled) | |||
There was a problem hiding this comment.
nit: Let's make it true by default to be backwards compatible.
There was a problem hiding this comment.
Yeah, I can keep this TODO open for now. I want to keep it false for now to be able to test. I'll set it to true before merging.
pkg/core/stack_resources.go
Outdated
| *networking.Ingress, | ||
| error, | ||
| ) { | ||
|
|
||
| // If Per-stack Hostnames are disabled, no need to generate per stack ingresses | ||
| if !perStackHostnameEnabled { |
There was a problem hiding this comment.
This is only if segment is also false, because we want to generate segment ingresses regardless.
There was a problem hiding this comment.
Ah, I see. I'll try with that. Thanks.
|
Closing this in favor of #696 which implements a better solution. |
This makes per-stack hostnames configurable in the stackset.
This allows us to disable support for per-stack hostnames, which is required to solve the problem of duplication of hostnames when a stackset is deployed in multiple namespaces, or multiple clusters in the same account.
The objective is to first be able to disable the support, and then encourage the users that actually rely on this to use
.cluster.localhostnames instead for their e2e testing.