Add scale subresource status to the OpenTelemetryCollector CRD status #785
Conversation
secat
force-pushed
the
775-add-scale-subresource-status
branch
from
e1c1bd9 to
8d548f3
Compare
| // +optional | ||
| Selector string `json:"selector,omitempty"` | ||
| } | ||
|
|
||
| // OpenTelemetryCollectorStatus defines the observed state of OpenTelemetryCollector. | ||
| type OpenTelemetryCollectorStatus struct { | ||
| // Replicas is currently not being set and might be removed in the next version. | ||
| // +optional | ||
| Replicas int32 `json:"replicas,omitempty"` |
There was a problem hiding this comment.
This should probably be deprecated and eventually removed, right?
There was a problem hiding this comment.
Exactly, should I remove it in this PR?
There was a problem hiding this comment.
This status field is currently unused
There was a problem hiding this comment.
Right, but I think it's still best to deprecate in one version, removing in another. Even if only to serve as a pattern for future, similar tasks.
There was a problem hiding this comment.
This is good, but for reference, we typically add the version it was deprecated, so that we know when it's safe to remove. Like:
// Deprecated: [v0.47.0] use "OpenTelemetryCollector.Status.Scale.Replicas" instead.
There was a problem hiding this comment.
As far as I know, properties from the CRD should not be removed. The cleanup can be done only when we promote the CRD to a new version.
secat
force-pushed
the
775-add-scale-subresource-status
branch
from
10d667e to
92b01c7
Compare
|
Just updated the |
|
Fixed also a |
|
@secat please take a look here #775 (comment) |
|
@pavolloffay and @jpkrohling is there anything that needs to be done prior to merge this PR? Thank you in advance! 😃 |
|
This is good to go for me, but given @pavolloffay's comments, it would be good to have an explicit approval from him. |
|
@pavolloffay anything missing in this PR? Thank you in advance |
|
I think this can go in as it is. I would maybe prefer to have the scale fields directly in the status to avoid deprecating |
…open-telemetry#785) * Add scale subresource status to the OpenTelemetryCollector CRD status * Deprecate OpenTelemetryCollector.Status.Replicas * Disable exhaustive linter on updateScaleSubResourceStatus 'switch mode'
Resolves #775