add forceTcp and keepSocket options #324
Conversation
| //+kubebuilder:default:=false | ||
| //+optional | ||
| ForceTcp bool `json:"forceTcp"` | ||
|
|
There was a problem hiding this comment.
force_tcp - false
keep_socket - true
| @@ -11,7 +11,9 @@ | |||
| chyt={ | |||
| "address_resolver"={ | |||
| "enable_ipv4"=%true; | |||
| "enable_ipv6"=%true; | |||
| "enable_ipv6"=%false; | |||
There was a problem hiding this comment.
cause of
- https://github.com/ytsaurus/ytsaurus-k8s-operator/pull/324/files#diff-9875b847729a36cdf9f8ee8333cefa406ed3e7fb1f5f297c87e5e3e556d907bbR558
- https://github.com/ytsaurus/ytsaurus-k8s-operator/pull/324/files#diff-ee85a92ac74a0dc4dbb84dab8829fe475be259f340407f523529d5dda26961ddR315-R317
and maybe it should be done another way
There was a problem hiding this comment.
So it was hardcoded for strawberry, but now it is configured according to the common value, ok
There was a problem hiding this comment.
On a second thought that may be backward incompatible change and maybe we need to think here more.
There was a problem hiding this comment.
So users, who have ipv4=false;ipv6=true or ipv4=true;ipv6=false (assume false/false is invalid configuration) will redeploy strawberry on operator update with this settings, not true/true as before which can break things for them. Not sure yet how problematic such update could be.
There was a problem hiding this comment.
I suppose we can describe breaking changes in the next release about strawberry ipv4/6.
|
Thanks, will look into it soon. |
| @@ -11,7 +11,9 @@ | |||
| chyt={ | |||
| "address_resolver"={ | |||
| "enable_ipv4"=%true; | |||
| "enable_ipv6"=%true; | |||
| "enable_ipv6"=%false; | |||
There was a problem hiding this comment.
So it was hardcoded for strawberry, but now it is configured according to the common value, ok
pkg/ytconfig/generator_test.go
Outdated
| forceTCP := true | ||
| keepSocket := true | ||
| ytsaurus.Spec.CommonSpec.ForceTCP = &forceTCP | ||
| ytsaurus.Spec.CommonSpec.KeepSocket = &keepSocket |
There was a problem hiding this comment.
ptr.To(..) can be used here for brevity, but it is minor.
pkg/ytconfig/generator.go
Outdated
| c := getStrawberryController() | ||
| var resolver AddressResolver | ||
| g.fillAddressResolver(&resolver) | ||
| c := getStrawberryController(&resolver) |
There was a problem hiding this comment.
Let's pass not pointer but struct to getStrawberryController since we are not changing it inside that function and nil value there is unexpected and will cause panic, so safer and more straightforward is just to just pass resolver value here.
|
Sent a commit with fixes |
No description provided.