Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate // +kubebuilder:default annotations to a defaulting webhook #1541

Open
skhalash opened this issue Oct 21, 2024 · 0 comments
Open

Migrate // +kubebuilder:default annotations to a defaulting webhook #1541

skhalash opened this issue Oct 21, 2024 · 0 comments
Assignees
Labels
area/logs LogPipeline area/metrics MetricPipeline area/traces TracePipeline

Comments

@skhalash
Copy link
Collaborator

skhalash commented Oct 21, 2024

Description

Introduce a defaulting webhook for all 3 types of pipelines and migrate away from // +kubebuilder:default annotations.

Reasons

Currently, we use // +kubebuilder:default annotations to set default values, but this approach has several limitations:

  • The defaulting mechanism only triggers when the immediate parent of the field with the default annotation is explicitly set in the manifest. However, we need to apply defaults regardless of the parent’s presence. To achieve this, we end up duplicating code at each level, as seen in the following example:
// MetricPipelineRuntimeInput defines the runtime scraping section.
type MetricPipelineRuntimeInput struct {
	// If enabled, runtime metrics are scraped. The default is `false`.
	Enabled bool `json:"enabled,omitempty"`
	// Describes whether runtime metrics from specific namespaces are selected. System namespaces are disabled by default.
	// +optional
	// +kubebuilder:default={exclude: {kyma-system, kube-system, istio-system, compass-system}}
	Namespaces *MetricPipelineInputNamespaceSelector `json:"namespaces,omitempty"`
	// Describes the Kubernetes resources for which runtime metrics are scraped.
	// +optional
	// +kubebuilder:default={pod: {enabled: true}, container: {enabled: true}, node: {enabled: false}, volume: {enabled: false}}
	Resources *MetricPipelineRuntimeInputResources `json:"resources,omitempty"`
}

// MetricPipelineRuntimeInputResources describes the Kubernetes resources for which runtime metrics are scraped.
type MetricPipelineRuntimeInputResources struct {
	// Configures Pod runtime metrics scraping.
	// +optional
	// +kubebuilder:default={enabled: true}
	Pod *MetricPipelineRuntimeInputResourceEnabledByDefault `json:"pod,omitempty"`
	// Configures container runtime metrics scraping.
	// +optional
	// +kubebuilder:default={enabled: true}
	Container *MetricPipelineRuntimeInputResourceEnabledByDefault `json:"container,omitempty"`
	// Configures Node runtime metrics scraping.
	// +optional
	// +kubebuilder:default={enabled: false}
	Node *MetricPipelineRuntimeInputResourceDisabledByDefault `json:"node,omitempty"`
	// Configures Volume runtime metrics scraping.
	// +optional
	// +kubebuilder:default={enabled: false}
	Volume *MetricPipelineRuntimeInputResourceDisabledByDefault `json:"volume,omitempty"`
}

// MetricPipelineRuntimeInputResourceEnabledByDefault defines if the scraping of runtime metrics is enabled for a specific resource. The scraping is enabled by default.
type MetricPipelineRuntimeInputResourceEnabledByDefault struct {
	// If enabled, the runtime metrics for the resource are scraped. The default is `true`.
	// +optional
	// +kubebuilder:default=true
	Enabled *bool `json:"enabled,omitempty"`
}
  • This setup requires full end-to-end tests, which are both slow and costly.
  • Common values like the system namespace list must be repeated across multiple places, leading to unnecessary code duplication.
  • We introduce unnecessary types like MetricPipelineRuntimeInputResourceEnabledByDefault and MetricPipelineRuntimeInputResourceDisabledByDefault, just to manage different sets of annotations.

An alternative approach is to use a defaulting webhook. While maintaining webhook infrastructure (e.g., certificates) can be costly, it's already in place for converting and validating webhooks, and is likely to stay in the future.

First Step:
Kustomize for Webhook Configurations: We can consider moving the ValidatingWebhookConfiguration from the operator code to Kustomize. The operator would then only patch the CA bundle, while the rest of the static configuration would be managed via Kustomize. The lifecycle would be controlled by a lifecycle manager. The same approach can be applied to the MutatingWebhookConfiguration for defaulting.

  • All webhook resources are part of kustomize (without caBundle section)
  • The manager only patches the resources with caBundle
  • There are no leftover resources after an upgrade to the new version
  • Validation should work as before, mutating logic gets triggered with NOOP

Second Step
Migrate all defaulting logic from CRDs to mutating webhooks

  • No kubebuilder annotations for defaulting present anymore
  • Defaulting log works as before
@skhalash skhalash added area/logs LogPipeline area/traces TracePipeline area/metrics MetricPipeline labels Oct 21, 2024
@a-thaler a-thaler mentioned this issue Oct 23, 2024
20 tasks
@hisarbalik hisarbalik self-assigned this Nov 5, 2024
@shorim shorim self-assigned this Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/logs LogPipeline area/metrics MetricPipeline area/traces TracePipeline
Projects
None yet
Development

No branches or pull requests

3 participants