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

Revert "add var disable_hetzner_ccm" #1596

Merged
merged 1 commit into from
Dec 24, 2024

Conversation

mysticaltech
Copy link
Collaborator

Reverts #1575

@mysticaltech
Copy link
Collaborator Author

Reverting PR #1575: Fixing Deployment Bug in Hetzner CCM

PR #1575 introduced the disable_hetzner_ccm variable, but it inadvertently caused a bug in the Hetzner Cloud Controller Manager (CCM) deployment. Here's why this PR is being reversed:

Original Design (Pre-PR #1575):

  • The remote ccm-networks.yaml was included as a resource.
  • The local ccm.yaml was used as a patch for customization.
  • This pattern ensured flexibility while maintaining the correct kustomize structure.

Issue Introduced by PR #1575:

  • Both ccm-networks.yaml and ccm.yaml are now included as resources.
  • This creates a conflict, as both define the same Kubernetes resource: Deployment.v1.apps/hcloud-cloud-controller-manager in the kube-system namespace.
  • The local ccm.yaml is still referenced as a patch, adding further confusion.

Why This is Problematic:

  • Double Deployment: Two attempts to deploy the same resource.
  • Broken Pattern: The kustomize resource/patch approach is disrupted.
  • Increased Complexity: Configuration becomes harder to manage and error-prone.

While the intent to enable CCM disabling was valid, the implementation broke the existing kustomize pattern and introduced a conflict. This reversion restores the original structure, ensuring stability and clarity.

If CCM disabling is needed, it should be implemented while preserving the resource/patch pattern to avoid similar issues.

@mysticaltech mysticaltech merged commit 4a0390e into master Dec 24, 2024
2 of 3 checks passed
@mysticaltech mysticaltech deleted the revert-1575-feat/disable-hccm branch December 24, 2024 08:01
@pat-s
Copy link
Contributor

pat-s commented Jan 2, 2025

@mysticaltech Just saw this now. Thanks for the extensive description.

The patch works for my use cases and adapting it to the structure of this module including the kustomize logic is probably not worth the effort. At least it would cost me a lot of time.

However, overall I'd argue that the option is pretty important as without this one cannot deploy HCCCM via helm (which is required when full compatibility is sought for robot servers). The helm variant does some magic behind the scenes which the kustomize one can't.

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.

2 participants