Set probes port and scheme based on .spec.manageTLS #735
Conversation
api/v1/runtimecomponent_types.go
Outdated
| } | ||
|
|
||
| func (cr *RuntimeComponent) GetManagedScheme() corev1.URIScheme { | ||
| if cr.GetManageTLS() != nil && *cr.GetManageTLS() { |
There was a problem hiding this comment.
Handle the default value of manageTLS (when nil)
There was a problem hiding this comment.
Added check for when GetManageTLS() == nil
api/v1/runtimecomponent_types.go
Outdated
| } | ||
|
|
||
| func (cr *RuntimeComponent) GetManagedPort() int { | ||
| return int(cr.GetService().GetPort()) |
There was a problem hiding this comment.
Validate that the default value Operator sets (when user hasn't configured Service port) is retrieved here.
There was a problem hiding this comment.
Added nil check on GetService() and fallback to 8080
common/types.go
Outdated
|
|
||
| // This struct is taken from the Probe specification in https://github.com/kubernetes/api/blob/v0.33.4/core/v1/types.go | ||
| // +kubebuilder:object:generate=true | ||
| type BaseComponentProbe struct { |
There was a problem hiding this comment.
Removed the structs
common/common.go
Outdated
| if probe.ProbeHandler.HTTPGet == nil { | ||
| probe.ProbeHandler.HTTPGet = &corev1.HTTPGetAction{} | ||
| } | ||
| if config.ProbeHandler.HTTPGet.Port.Type != 0 { |
There was a problem hiding this comment.
Just wanted to confirm that a nil check on config.ProbeHandler.HTTPGet.Port is not needed here.
There was a problem hiding this comment.
Thanks, it is not needed, it is an artifact from prior utils_test.go changes.
common/common.go
Outdated
| Port: intstr.FromInt(int(ba.GetService().GetPort())), | ||
| Scheme: "HTTPS", | ||
| Port: intstr.FromInt(ba.GetManagedPort()), | ||
| Scheme: corev1.URISchemeHTTPS, |
There was a problem hiding this comment.
If the Service port is 9080 then we should use HTTP scheme instead, so that users wouldn't have to add config just to specify the HTTP scheme.
There was a problem hiding this comment.
Sure, some tests assume scheme is not affected by .spec.manageTLS so there may be some breaking changes. For example, some probes tests need adjustment after this change. Please see adjustments at https://github.ibm.com/websphere/operators/pull/258/files#diff-d8a058f0da1d4a6c12d831cf53afe614d6e03adecc6b633a4a387d4294f239e2 for added context.
kabicin
changed the title
kabicin
changed the title
What this PR does / why we need it?:
.spec.manageTLSvalue. For example, if manageTLS is true or undefined, scheme will be HTTPS and otherwise, HTTP.Does this PR introduce a user-facing change?
CHANGELOG.mdWhich issue(s) this PR fixes:
Fixes #