Skip to content

feat(traits): remove deprecated logging, master and telemetry traits - #6807

Open
hxxxi-malog wants to merge 9 commits into
apache:mainfrom
hxxxi-malog:feature/6805-drop-deprecated-traits
Open

hxxxi-malog wants to merge 9 commits into
apache:mainfrom
hxxxi-malog:feature/6805-drop-deprecated-traits

Conversation

@hxxxi-malog

@hxxxi-malog hxxxi-malog commented Sep 13, 2026

Copy link
Copy Markdown

Fixes #6805

Removes the logging, master and telemetry traits, deprecated since 2.9.0, following the removal pattern used for the Jolokia trait (#6764).

What is removed

  • Trait runtime implementations, registrations and unit tests (pkg/trait/logging.go, master.go, telemetry.go)
  • Jaeger discovery support under pkg/trait/discovery/, which was only used by the telemetry trait
  • Master RBAC embedded templates under pkg/resources/resources/addons/master/; RBAC is no longer created automatically by the operator
  • The resources.TemplateResource helper, its test and its master template fixture, because they were only used by the removed master trait
  • Trait documentation pages and navigation entries

Generated CRDs, Helm CRDs, deepcopy code, apply configurations and API documentation are updated to match the remaining deprecated API fields.

Compatibility and behavior

  • The deprecated spec.traits fields remain in the CRD/API with their original JSON names: logging, master and telemetry
  • The corresponding Go fields are retained as DeprecatedLogging, DeprecatedMaster and DeprecatedTelemetry, following the existing removed-trait pattern
  • Configurations for removed traits in spec.traits have no active implementation and are silently ignored
  • Legacy annotations for removed traits receive no special carve-out: annotations for traits absent from the catalog continue to fail validation, as before
  • No existing master Role or RoleBinding is automatically deleted by this change

Migration

  • logging: traits.logging.level=DEBUG -> -p quarkus.log.level=DEBUG
  • telemetry: --trait telemetry.endpoint=http://jaeger:4317 -> -p quarkus.otel.exporter.otlp.traces.endpoint=http://jaeger:4317
  • master: create the required Role and RoleBinding manually, then configure:
    -p quarkus.camel.cluster.kubernetes.resource-name=<integration>-lock
    -p quarkus.camel.cluster.kubernetes.resource-type=Lease
    -p quarkus.camel.cluster.kubernetes.labels."camel.apache.org/integration"=<integration-name>

For existing Integrations using the master trait, create the manual RBAC before removing the trait configuration.

Tests and verification

  • Existing unit tests that referenced removed traits now use active traits; the merge test uses the Cron trait
  • E2E log color configuration now uses the Quarkus property quarkus.console.color
  • Affected package tests pass:
    go test ./pkg/trait ./pkg/apis/camel/v1 ./pkg/apis/camel/v1/trait ./pkg/client/camel/applyconfiguration/camel/v1 ./pkg/resources ./pkg/controller/pipe ./pkg/cmd ./pkg/platform
  • Affected package vet checks pass
  • Integration-tagged E2E packages compile successfully:
    go test -tags=integration -run "^\$" ./e2e/...
  • Modified Go files are gofmt-clean and git diff --check passes

AI-generated by Codex on behalf of @hxxxi-malog; reviewed and completed by @hxxxi-malog.

@hxxxi-malog
hxxxi-malog marked this pull request as draft September 14, 2026 07:06
@hxxxi-malog
hxxxi-malog marked this pull request as ready for review September 14, 2026 07:17

@squakez squakez left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, thanks for the work!

Comment thread pkg/resources/resources/testdata/template.tmpl Outdated
Comment thread pkg/resources/resources_test.go Outdated
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Unit test coverage report - coverage decreased from 63.2% to 63.1% (-0.1%)

@hxxxi-malog

Copy link
Copy Markdown
Author

Now, TemplateResource(), its test and the template.tmpl fixture are all gone.

@squakez squakez left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the fix. I've gone through more carefully and there are a few things to change.

Comment thread pkg/apis/camel/v1/common_types_support_test.go Outdated
Comment thread pkg/apis/camel/v1/common_types_support_test.go Outdated
Comment thread pkg/controller/pipe/integration_test.go Outdated
Comment thread pkg/trait/trait_configure.go Outdated
Comment thread pkg/trait/trait_configure_test.go Outdated
Comment thread pkg/trait/util.go Outdated
Comment thread pkg/trait/util.go
Comment thread pkg/trait/util_test.go Outdated
Comment thread pkg/trait/util_test.go Outdated
@hxxxi-malog

Copy link
Copy Markdown
Author

Thank you for your review. I have now reverted the special handling in ExtractAndMaybeDeleteTraits and configureTrait, removed the removed-trait-specific tests, and changed TestTraitsMerge to use the cron trait.

@squakez squakez left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great work, thanks!

@squakez

squakez commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Pipe test is failing with

[6](https://github.com/apache/camel-k/actions/runs/34865831079/job/104267087651?pr=6807#step:7:366)
❌ TestPipe (12.41s)
      pipe_test.go:59: 
          Expected success, but got an error:
              <*errors.errorString | 0x176aec23b7b0>: 
              property key "quarkus.console.color" does not start with "source.", "sink.", "error-handler." or "step-<n>."
              {
                  s: "property key \"quarkus.console.color\" does not start with \"source.\", \"sink.\", \"error-handler.\" or \"step-<n>.\"",
              }

I think that the -p in kamel bind has different logic that in kamel run.

@squakez squakez left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The proposed fix should clear the error.

Comment thread e2e/common/misc/pipe_test.go Outdated
Comment thread e2e/common/misc/pipe_test.go Outdated
@squakez

squakez commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

@hxxxi-malog please, rebase. Once done if all checks are green we can merge.

@hxxxi-malog

hxxxi-malog commented Sep 16, 2026

Copy link
Copy Markdown
Author

@hxxxi-malog please, rebase. Once done if all checks are green we can merge.请重新处理一下。一旦所有检查都通过,我们就可以进行合并了。

Thanks for your review. The validate check on main is also red at run.go, and it looks like it was introduced by the new gofmt addition. How should this be handled?

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Unit test coverage report - coverage decreased from 63.8% to 63.6% (-0.2%)

@squakez

squakez commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

@hxxxi-malog please, rebase. Once done if all checks are green we can merge.请重新处理一下。一旦所有检查都通过,我们就可以进行合并了。

Thanks for your review. The validate check on main is also red at run.go, and it looks like it was introduced by the new gofmt addition. How should this be handled?

Ah, yeah, sorry, that is some wrong formatting that I'm going to fix it in main right away. Won't be a blocker for merging this PR.

@squakez

squakez commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

@hxxxi-malog should be fixed on main now. Feel free to rebase

hxxxi-malog and others added 6 commits September 16, 2026 23:13
- Ignore legacy trait annotations of the removed logging, master and
  telemetry traits during extraction (cleaning them up when migrating),
  while still rejecting other unknown traits
- Restore the exported resources.TemplateResource API that was removed
  with the master trait, with a neutral test template
- Extend removed-trait test coverage: table-driven configure tests for
  all three traits, legacy JSON field names, merge behavior and the
  annotation paths for Integration, IntegrationKit and Pipe
- Regenerate deepcopy artifacts with the pinned controller-gen v0.20.0
  so generated style matches CI output
…aits

Revert ExtractAndMaybeDeleteTraits and Catalog.configureTrait to the
previous logic so removed traits are treated exactly like any other
trait missing from the catalog, and drop the tests that were specific
to removed-trait behavior. TestTraitsMerge now uses the cron trait
instead of the deprecated logging one.
@hxxxi-malog
hxxxi-malog force-pushed the feature/6805-drop-deprecated-traits branch 2 times, most recently from 3b92b6d to 0fb3e88 Compare September 16, 2026 15:21
@hxxxi-malog

Copy link
Copy Markdown
Author

Thanks @squakez ! The rebase is done, the branch is now on top of the latest main, and both camel.properties suggestions have been included.

@squakez

squakez commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

E2E master test has to be removed as well. Not sure if there is also logging left over.

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Unit test coverage report - coverage decreased from 63.8% to 63.6% (-0.2%)

@hxxxi-malog

Copy link
Copy Markdown
Author

E2E master test has to be removed as well. Not sure if there is also logging left over.

Thanks, I'll remove the e2e master test. As for logging leftovers, I couldn't find any — there is no logging-specific e2e test, and the remaining loggerName usages are Kamelet configuration unrelated to the trait. The DeprecatedLogging field in pkg/apis/camel/v1/trait/logging.go is kept intentionally, as required by the deprecated API compatibility discussed earlier.

One question: e2e/telemetry/telemetry_test.go (TestTelemetryTrait) is in the same situation — it no longer uses the trait and instead configures quarkus.otel.* properties directly. Shall I remove it as well?

@squakez

squakez commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Telemetry test needs to stay as they are testing telemetries via the new mechanism, not involving older trait. Thanks!

@hxxxi-malog

Copy link
Copy Markdown
Author

Done, the master e2e test has been removed in the commit, and its dedicated route fixture Master.java has also been deleted, as it is no longer referenced anywhere else.

@squakez

squakez commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

@hxxxi-malog sorry, I made a mistake. Just double checking and the master test was fine, I mean, it did not reference already any master trait. It must stay, and, if it failed, we need to verify why. Please revert the related removal and we can run another round of test to validate what was really going on. My apologies for the confusion.

@hxxxi-malog

Copy link
Copy Markdown
Author

OK, the master e2e test and its fixture have been reverted, with content identical to main.

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Unit test coverage report - coverage decreased from 63.8% to 63.6% (-0.2%)

@hxxxi-malog

Copy link
Copy Markdown
Author

I think the reason the check failed is that the deleted master trait is still automatically activated on main and adds the master capability to the Integration. During the build, this is used to package the cluster service library into the image. After removing it, the image is missing that library, so Quarkus ignores those three properties, and the master: endpoint reports "No cluster service found". The solution is to explicitly add the dependency in the migration path: -d mvn:org.apache.camel.quarkus:camel-quarkus-kubernetes-cluster-service. Is it okay to make the change in this direction?

@squakez

squakez commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

I think the reason the check failed is that the deleted master trait is still automatically activated on main and adds the master capability to the Integration. During the build, this is used to package the cluster service library into the image. After removing it, the image is missing that library, so Quarkus ignores those three properties, and the master: endpoint reports "No cluster service found". The solution is to explicitly add the dependency in the migration path: -d mvn:org.apache.camel.quarkus:camel-quarkus-kubernetes-cluster-service. Is it okay to make the change in this direction?

Yeah, it seems it is the correct diagnose. Please, add the related dependency into the test and we can restart a round of test, thanks!

@hxxxi-malog

Copy link
Copy Markdown
Author

OK, I've finished adding the dependency to every KamelRun in the master e2e test, and now we can start testing to see how it works.

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Unit test coverage report - coverage decreased from 63.8% to 63.6% (-0.2%)

@hxxxi-malog

Copy link
Copy Markdown
Author

Hmm... it looks like the last dependency fix has taken effect, but the test still fails because the trait has revealed another hidden responsibility: the deleted trait would parse URIs like master:lock:timer:tick and add the inner camel:timer component to the dependencies. I'll add -d camel:timer in the test; also, the resource-type=Lease property is no longer valid in the new runtime. Does this approach sound feasible?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Drop 2.9 deprecated traits

2 participants