Remove v1alpha1 webhooks #2861
Remove v1alpha1 webhooks #2861
Conversation
pavolloffay
force-pushed
the
v1beta1-remove-webhook
branch
from
6d933ab to
ee90473
Compare
| @@ -50,47 +86,353 @@ func (c CollectorWebhook) Default(_ context.Context, obj runtime.Object) error { | |||
| if !ok { | |||
| return fmt.Errorf("expected an OpenTelemetryCollector, received %T", obj) | |||
| } | |||
| return c.defaulter(otelcol) | |||
| if len(otelcol.Spec.Mode) == 0 { | |||
There was a problem hiding this comment.
can we move this logic to that defaulter method?
There was a problem hiding this comment.
I am not sure what you mean, this is the Default method.
There was a problem hiding this comment.
i meant keeping this in the internal method (defaulter), but i guess it doesn't really matter
There was a problem hiding this comment.
I didn't see the point of the internal defaulter method
| @@ -44,7 +43,7 @@ func (c ComponentType) String() string { | |||
| } | |||
|
|
|||
| // ConfigToComponentPorts converts the incoming configuration object into a set of service ports required by the exporters. | |||
| func ConfigToComponentPorts(logger logr.Logger, cType ComponentType, config map[interface{}]interface{}) ([]v1beta1.PortsSpec, error) { | |||
| func ConfigToComponentPorts(logger logr.Logger, cType ComponentType, config map[interface{}]interface{}) ([]corev1.ServicePort, error) { | |||
There was a problem hiding this comment.
I think we should be using the ports spec here right?
There was a problem hiding this comment.
I needed to change this back to corev1.ServicePort to avoid a circular dependency.
Originally it was using the type from corev1 but it was changed here https://github.com/open-telemetry/opentelemetry-operator/pull/2763/files#diff-39548fb5f0030535966b3fb437c08814c648216539e297c034f3da6ac5b43a9bR47
There was a problem hiding this comment.
doing this is going to make it very hard to complete #2603 because we want to be able to use those types. Is it possible to avoid this by moving the methods?
pavolloffay
force-pushed
the
v1beta1-remove-webhook
branch
from
ea7f12d to
4226f74
Compare
| ctrl "sigs.k8s.io/controller-runtime" | ||
| "sigs.k8s.io/controller-runtime/pkg/webhook/admission" | ||
|
|
||
| "github.com/open-telemetry/opentelemetry-operator/internal/config" | ||
| ta "github.com/open-telemetry/opentelemetry-operator/internal/manifests/targetallocator/adapters" |
There was a problem hiding this comment.
this is unfortunate – I would love it if there were a way out of this conundrum... I don't think that needs to be in this PR but is related to the work I'm doing in #2603. Could you open a ticket and write a TODO here linking it for us to prevent pulling from the manifests package
There was a problem hiding this comment.
Do you mean moving the methods from internal/manifests/collector/adapters/config_to_ports.go to the API package?
There was a problem hiding this comment.
yeah... something like that... I think with the new capabilities from the v1beta1 change we really may want to rethink how we can handle all of these adapters.
pavolloffay
force-pushed
the
v1beta1-remove-webhook
branch
from
335a11a to
2eafde3
Compare
Signed-off-by: Pavol Loffay <[email protected]>
Signed-off-by: Pavol Loffay <[email protected]>
pavolloffay
force-pushed
the
v1beta1-remove-webhook
branch
from
2eafde3 to
9fd0e9b
Compare
pavolloffay
added
the
Skip Changelog
Signed-off-by: Pavol Loffay <[email protected]>
pavolloffay
force-pushed
the
v1beta1-remove-webhook
branch
from
a83b23d to
d46ff95
Compare
| @@ -202,7 +202,7 @@ func getConfigContainerPorts(logger logr.Logger, cfgYaml string, conf v1beta1.Co | |||
| } | |||
| } | |||
|
|
|||
| metricsPort, err := adapters.ConfigToMetricsPort(conf.Service) | |||
| metricsPort, err := conf.Service.MetricsPort() | |||
* Remove v1alpha1 webhooks Signed-off-by: Pavol Loffay <[email protected]> * Fix Signed-off-by: Pavol Loffay <[email protected]> * Fix Signed-off-by: Pavol Loffay <[email protected]> --------- Signed-off-by: Pavol Loffay <[email protected]>
* Remove v1alpha1 webhooks Signed-off-by: Pavol Loffay <[email protected]> * Fix Signed-off-by: Pavol Loffay <[email protected]> * Fix Signed-off-by: Pavol Loffay <[email protected]> --------- Signed-off-by: Pavol Loffay <[email protected]>
Resolves #2736