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

Enhanced Webhook Reconciliation Errors #3180

Merged
merged 49 commits into from
Sep 5, 2024
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
04942af
initial commit
taniyourstruly Jun 25, 2024
ffa2407
added enhanced reconciliation errors
taniyourstruly Jul 15, 2024
8c59958
tests
taniyourstruly Jul 15, 2024
f80e75e
update
taniyourstruly Jul 17, 2024
64b7730
tests
taniyourstruly Jul 18, 2024
5f290a9
add test
taniyourstruly Jul 29, 2024
72acf24
changelog
taniyourstruly Jul 29, 2024
348dcdb
initial fixes
taniyourstruly Jul 29, 2024
1f420d8
linting
taniyourstruly Jul 29, 2024
0eae73d
changing to warnings
taniyourstruly Jul 30, 2024
2964394
merge conflicts
taniyourstruly Jul 30, 2024
55f7488
shadow declarations
taniyourstruly Jul 30, 2024
e57985b
fixing errors
taniyourstruly Jul 30, 2024
b594d97
kubebuilder test
taniyourstruly Jul 30, 2024
2fe4630
kubebuilder testing
taniyourstruly Jul 30, 2024
32bac14
fixing file
taniyourstruly Jul 30, 2024
69c39d6
unit test
taniyourstruly Jul 30, 2024
b070c5f
Merge branch 'main' into enhanced-webhook
taniyourstruly Aug 6, 2024
895c802
initial changes
taniyourstruly Aug 7, 2024
f620524
clean up
taniyourstruly Aug 12, 2024
59f50c1
merge conflicts
taniyourstruly Aug 12, 2024
a93594c
Update apis/v1beta1/collector_webhook_test.go
taniyourstruly Aug 13, 2024
a1d71f2
fixed test
taniyourstruly Aug 13, 2024
1f5442b
fixed test
taniyourstruly Aug 13, 2024
5a6387c
merge conflicts
taniyourstruly Aug 13, 2024
f95327f
merge conflicts
taniyourstruly Aug 13, 2024
33fbc03
unneeded code
taniyourstruly Aug 13, 2024
f8fb046
linting
taniyourstruly Aug 13, 2024
65477b5
linting and unit tests
taniyourstruly Aug 13, 2024
3fdc89f
reuse code
taniyourstruly Aug 15, 2024
2fc8b80
Merge branch 'main' of github.com:open-telemetry/opentelemetry-operat…
taniyourstruly Aug 15, 2024
eb5eabf
Merge branch 'main' into enhanced-webhook
taniyourstruly Aug 15, 2024
8e7282c
Update apis/v1beta1/collector_webhook.go
taniyourstruly Aug 16, 2024
5dcfa35
Merge branch 'enhanced-webhook' of https://github.com/taniyourstruly/…
taniyourstruly Aug 16, 2024
b6f0911
revert to not use getParams
taniyourstruly Aug 29, 2024
56e4402
use config
taniyourstruly Aug 29, 2024
df46a05
linting
taniyourstruly Aug 29, 2024
4289b44
more linting
taniyourstruly Aug 29, 2024
42f9a1f
validate as anon func
taniyourstruly Sep 4, 2024
eab4fd1
linting
taniyourstruly Sep 4, 2024
a4f19e2
linting
taniyourstruly Sep 4, 2024
68d1686
testing without test
taniyourstruly Sep 4, 2024
f7f571b
added back test
taniyourstruly Sep 4, 2024
bdd2d11
Merge branch 'main' into enhanced-webhook
jaronoff97 Sep 5, 2024
616726e
fix
jaronoff97 Sep 5, 2024
9707061
Merge pull request #1 from jaronoff97/enhanced-webhook
taniyourstruly Sep 5, 2024
c552bd0
make generate
taniyourstruly Sep 5, 2024
b01f894
Merge branch 'main' of github.com:open-telemetry/opentelemetry-operat…
taniyourstruly Sep 5, 2024
52eda8a
merge conflicts
taniyourstruly Sep 5, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .chloggen/enhanced-webhook.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement

# The name of the component, or a single word describing the area of concern, (e.g. operator, target allocator, github action)
component: operator

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: added reconciliation errors for webhook events
taniyourstruly marked this conversation as resolved.
Show resolved Hide resolved

# One or more tracking issues related to the change
issues: [2399]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:
41 changes: 35 additions & 6 deletions apis/v1beta1/collector_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ type CollectorWebhook struct {
scheme *runtime.Scheme
reviewer *rbac.Reviewer
metrics *Metrics
bv BuildValidator
}

func (c CollectorWebhook) Default(_ context.Context, obj runtime.Object) error {
Expand Down Expand Up @@ -141,14 +142,17 @@ func (c CollectorWebhook) ValidateCreate(ctx context.Context, obj runtime.Object
return nil, fmt.Errorf("expected an OpenTelemetryCollector, received %T", obj)
}

warnings, err := c.validate(ctx, otelcol)
warnings, err := c.Validate(ctx, otelcol)
if err != nil {
return warnings, err
}
if c.metrics != nil {
c.metrics.create(ctx, otelcol)
}

if c.bv != nil {
newWarnings := c.bv(*otelcol)
warnings = append(warnings, newWarnings...)
}
return warnings, nil
}

Expand All @@ -166,7 +170,7 @@ func (c CollectorWebhook) ValidateUpdate(ctx context.Context, oldObj, newObj run
if otelcolOld.Spec.Mode != otelcol.Spec.Mode {
return admission.Warnings{}, fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support modification", otelcolOld.Spec.Mode)
}
warnings, err := c.validate(ctx, otelcol)
warnings, err := c.Validate(ctx, otelcol)
if err != nil {
return warnings, err
}
Expand All @@ -175,6 +179,10 @@ func (c CollectorWebhook) ValidateUpdate(ctx context.Context, oldObj, newObj run
c.metrics.update(ctx, otelcolOld, otelcol)
}

if c.bv != nil {
newWarnings := c.bv(*otelcol)
warnings = append(warnings, newWarnings...)
}
return warnings, nil
}

Expand All @@ -184,7 +192,7 @@ func (c CollectorWebhook) ValidateDelete(ctx context.Context, obj runtime.Object
return nil, fmt.Errorf("expected an OpenTelemetryCollector, received %T", obj)
}

warnings, err := c.validate(ctx, otelcol)
warnings, err := c.Validate(ctx, otelcol)
if err != nil {
return warnings, err
}
Expand All @@ -196,7 +204,7 @@ func (c CollectorWebhook) ValidateDelete(ctx context.Context, obj runtime.Object
return warnings, nil
}

func (c CollectorWebhook) validate(ctx context.Context, r *OpenTelemetryCollector) (admission.Warnings, error) {
func (c CollectorWebhook) Validate(ctx context.Context, r *OpenTelemetryCollector) (admission.Warnings, error) {
warnings := admission.Warnings{}

nullObjects := r.Spec.Config.nullObjects()
Expand Down Expand Up @@ -437,13 +445,34 @@ func checkAutoscalerSpec(autoscaler *AutoscalerSpec) error {
return nil
}

func SetupCollectorWebhook(mgr ctrl.Manager, cfg config.Config, reviewer *rbac.Reviewer, metrics *Metrics) error {
// BuildValidator is mostly used for testing purposes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a real description here for the purpose of this type?

// +kubebuilder:object:generate=false
type BuildValidator func(c OpenTelemetryCollector) admission.Warnings

func NewCollectorWebhook(
taniyourstruly marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you note that this is mostly for testing

logger logr.Logger,
scheme *runtime.Scheme,
cfg config.Config,
reviewer *rbac.Reviewer,
bv BuildValidator,
) *CollectorWebhook {
return &CollectorWebhook{
logger: logger,
scheme: scheme,
cfg: cfg,
reviewer: reviewer,
bv: bv,
}
}

func SetupCollectorWebhook(mgr ctrl.Manager, cfg config.Config, reviewer *rbac.Reviewer, metrics *Metrics, bv BuildValidator) error {
cvw := &CollectorWebhook{
taniyourstruly marked this conversation as resolved.
Show resolved Hide resolved
reviewer: reviewer,
logger: mgr.GetLogger().WithValues("handler", "CollectorWebhook", "version", "v1beta1"),
scheme: mgr.GetScheme(),
cfg: cfg,
metrics: metrics,
bv: bv,
}
return ctrl.NewWebhookManagedBy(mgr).
For(&OpenTelemetryCollector{}).
Expand Down
Loading
Loading