Fix: tenant-root domain always overrided to example.org - #10
Merged
Conversation
Andrei Kvapil (kvaps)
requested review from
Eduard Generalov (egeneralov) and
George Gaál (gecube)
February 9, 2024 11:59
Andrei Kvapil (kvaps)
force-pushed
the
fix-root-domain
branch
from
February 9, 2024 12:14
28577bc to
b532b97
Compare
György Gaál (gaalw)
approved these changes
Feb 9, 2024
|
|
||
| show: | ||
| helm template -n $(NAMESPACE) $(NAME) . --dry-run=server | ||
| helm template -n $(NAMESPACE) $(NAME) . --dry-run=server $$(kubectl api-versions | awk '{print "-a " $$1}') |
There was a problem hiding this comment.
I don't like interpolations with
$(bash_command)
or
`(bash_command)`
as they can lead to very difficult to debug errors. The better approach is to use xargs:
kubectl api-versions | awk '{print "-a " $$1}' | xargs -r -I{} helm template -n $(NAMESPACE) $(NAME) . --dry-run=server {}in case if awk will return empty string, xargs won't run at all.
There was a problem hiding this comment.
like bonus - you don't need to use awkward syntax like double dollar
Member
Author
There was a problem hiding this comment.
yeah, but you still need it for awk :)
Member
Author
There was a problem hiding this comment.
I still have concerns about using xargs it might work diferent on various systems, awk is always the same.
There was a problem hiding this comment.
Not 100% accurate, you have gawk and mawk :)
George Gaál (gecube)
approved these changes
Feb 9, 2024
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
Andrei Kvapil (kvaps)
force-pushed
the
fix-root-domain
branch
from
February 9, 2024 12:19
b532b97 to
ddb4682
Compare
Member
Author
|
George Gaál (@gecube) I rebased the PR to use pure Makefile, without any |
This was referenced Aug 4, 2025
7 tasks
Matthieu ROBIN (matthieu-robin)
pushed a commit
to matthieu-robin/cozystack
that referenced
this pull request
May 17, 2026
Following kvaps' review, move the ZK→KRaft migration logic out of the chart templates and into a pre-upgrade Job gated by a <release>-kafka-deployed-version ConfigMap (seaweedfs/etcd pattern). The chart now ships pure KRaft on day one: - Kafka CR always renders with strimzi.io/kraft=enabled and no spec.zookeeper block. - All lookup-based state detection is removed from kafka.yaml, kafkanodepools.yaml, metrics-configmap.yaml, workloadmonitor.yaml and dashboard-resourcemap.yaml. - The zookeeper: block is dropped from values.yaml, values.schema.json, types.go and the kafka-rd openAPISchema. Migration of existing ZK clusters is handled by a new templates/migration-hook.yaml that renders only when the version ConfigMap is missing or below "1". The Job creates broker + controller KafkaNodePools (with Helm ownership labels so the subsequent chart apply can adopt them), annotates the Kafka CR with strimzi.io/node-pools=enabled and strimzi.io/kraft=migration, polls status.kafkaMetadataState until KRaftPostMigration or later, flips the annotation to enabled and waits for KRaft. Deviation from kvaps' proposal: separate broker+controller pools instead of a combined pool. Strimzi 0.45 does not support migration into a combined pool, so a homogeneous separate-pool layout works for both fresh installs and migrations without conditional logic. Other review fixes: - _versions.tpl: sortAlpha on the allowed-versions error message (cozystack#10). - e2e: rebased on main (event-driven kubectl waits), drops zookeeper readiness checks. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Matthieu ROBIN (matthieu-robin)
pushed a commit
to matthieu-robin/cozystack
that referenced
this pull request
May 17, 2026
Following kvaps' review, move the ZK→KRaft migration logic out of the chart templates and into a pre-upgrade Job gated by a <release>-kafka-deployed-version ConfigMap (seaweedfs/etcd pattern). The chart now ships pure KRaft on day one: - Kafka CR always renders with strimzi.io/kraft=enabled and no spec.zookeeper block. - All lookup-based state detection is removed from kafka.yaml, kafkanodepools.yaml, metrics-configmap.yaml, workloadmonitor.yaml and dashboard-resourcemap.yaml. - The zookeeper: block is dropped from values.yaml, values.schema.json, types.go and the kafka-rd openAPISchema. Migration of existing ZK clusters is handled by a new templates/migration-hook.yaml that renders only when the version ConfigMap is missing or below "1". The Job creates broker + controller KafkaNodePools (with Helm ownership labels so the subsequent chart apply can adopt them), annotates the Kafka CR with strimzi.io/node-pools=enabled and strimzi.io/kraft=migration, polls status.kafkaMetadataState until KRaftPostMigration or later, flips the annotation to enabled and waits for KRaft. Deviation from kvaps' proposal: separate broker+controller pools instead of a combined pool. Strimzi 0.45 does not support migration into a combined pool, so a homogeneous separate-pool layout works for both fresh installs and migrations without conditional logic. Other review fixes: - _versions.tpl: sortAlpha on the allowed-versions error message (cozystack#10). - e2e: rebased on main (event-driven kubectl waits), drops zookeeper readiness checks. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Matthieu <matthieu@hidora.com>
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
.Capabilities.APIVersions.Has "helm.toolkit.fluxcd.io/v2beta1does not work for Helm Template, so we must explicity pass apiversions tohelm templatecommandsee: helm/helm#10760