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

ENTESB-14406 Provide metering labels for Camel K Operator and deployed pods #56

Conversation

claudio4j
Copy link

https://issues.redhat.com/browse/ENTESB-14406

  • See the linked issue for context
  • As the metering labels are mandatory for any running pod, I found simpler to have it straight on deployment/cron/knative trait
  • This is a draft for early review before finishing the full work

@claudio4j claudio4j marked this pull request as draft September 16, 2021 02:06
@astefanutti
Copy link

The downside I see with the current approach is that the product / metering labels defined in the code must be re-applied for each new product branch. Ideally, these would be defined outside of the operator code, so that a new product branch can be created from the corresponding branch, without additional patches, and customisation script run in product build can be re-applied as is.

@claudio4j
Copy link
Author

I thought about that, perhaps a more complex approach can inject generated code, but that would be more complex.
The versions defined in Makefile (VERSION and RH_INTEGRATION_PRODUCT_VERSION) should be changed anyway, but only at the time of release. And this is the only part the release/developer should care about metering label versions.
The only thing I am uncertain is the placing of adjust-metering-labels in the release target of Makefile, WDYT ?

@astefanutti
Copy link

Intuitively, the logic that adds labels to controllers (Deployment, KnativeService, CronJob) would take a list of labels from a build time variable. That logic can be added upstream. Only the build time variable is set during the downstream build in https://github.com/jboss-fuse/fuse-camel-k, as we do for other product customisation. The customisation of the operator deployment can be done directly into the bundle https://github.com/jboss-fuse/fuse-camel-k-prod-operator-metadata/blob/integration-1.0/manifests/camel-k.clusterserviceversion.yaml#L293.

@claudio4j
Copy link
Author

This build time variable approach to change operator-deployment.yaml and deployment/knative/cron*.go will left uncommited files, then the productized process should commit them to the tag/branch along ?
For the yaml file it's ok to use bash scripts to change them, but for the .go files, using go:generate would be prefereable, but it is not allowed to run in productization process. What concerns me the most is the uncommited files being visible only in the repo tag.

@astefanutti
Copy link

This build time variable approach to change operator-deployment.yaml and deployment/knative/cron*.go will left uncommited files, then the productized process should commit them to the tag/branch along ?
For the yaml file it's ok to use bash scripts to change them, but for the .go files, using go:generate would be prefereable, but it is not allowed to run in productization process. What concerns me the most is the uncommited files being visible only in the repo tag.

No it's not about changing files. For the operator deployment, it's done with https://github.com/jboss-fuse/fuse-camel-k-prod-operator-metadata/pull/44. I don't think there is nothing else needed. For Pods labels, the idea is to have the mechanism upstream to add some labels based on a built time variable, like we already did for other product customisation: https://github.com/jboss-fuse/fuse-camel-k/blob/fuse-8.0-openshift-rhel-8/Dockerfile#L40. That left no uncommitted changes.

@claudio4j
Copy link
Author

makes sense, I will change the PR to take this approach. Thanks.

@claudio4j
Copy link
Author

I will close this in favor of the upstream one apache#2646

@claudio4j claudio4j closed this Sep 17, 2021
@claudio4j claudio4j deleted the feat_metering_labels branch September 22, 2021 14:26
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.

3 participants