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

Add helm hooks for admission controller deplolyment #30

Merged
merged 17 commits into from
Oct 15, 2024

Conversation

mlladb
Copy link
Contributor

@mlladb mlladb commented Oct 14, 2024

I've tested the following fix for the further issue described in Issue 25 at it works as expected. I don't think there are any likely issues to appear due to this change.

@eminaktas
Copy link
Owner

@mlladb Thanks for the PR. We want to reject DNSClass object if the validation fails. If we do this, we would skip that process.

I think we should add a DNSClass template inside the existing template and applying it with a hook so that it gets applied last. Would you be interested in contributing a commit for this feature?

@mlladb mlladb closed this Oct 15, 2024
@mlladb mlladb deleted the admissioncontroller-helmhook-weight branch October 15, 2024 01:08
@mlladb mlladb restored the admissioncontroller-helmhook-weight branch October 15, 2024 02:02
@mlladb mlladb reopened this Oct 15, 2024
@mlladb mlladb force-pushed the admissioncontroller-helmhook-weight branch 2 times, most recently from 93d7b1a to b57c2c2 Compare October 15, 2024 02:34
@mlladb
Copy link
Contributor Author

mlladb commented Oct 15, 2024

I've add the dnsclass templating. It seems to work as expected for me.
I can add my own custom config with the flowing values.yaml file.

kubedns-shepherd:
  certmanager:
    enabled: false
  dnsclass:
    allowedNamespaces:
      - default
      - kubedns-shepherd
    disabledNamespaces:
      - kube-system
    allowedDNSPolicies:
      - None
      - ClusterFirst
      - ClusterFirstWithHostNet
    dnsPolicy: None
    dnsConfig:
      nameservers:
        - 172.20.0.10
      searches:
        - "{{ .podNamespace }}.svc.cluster.local"
        - "svc.cluster.local"
        - "cluster.local"
      options: 
        - name: ndots
          value: "3"
        - name: edns0

Copy link
Owner

@eminaktas eminaktas left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution and validation—it’s highly appreciated. I’ve made a few suggestions. Please review them.

chart/kubedns-shepherd/values.yaml Show resolved Hide resolved
chart/kubedns-shepherd/values.yaml Show resolved Hide resolved
chart/kubedns-shepherd/values.yaml Show resolved Hide resolved
chart/kubedns-shepherd/values.yaml Show resolved Hide resolved
@eminaktas
Copy link
Owner

I also realized that your changes would work for Argo but when Helm applies the changes it will fail because it requires to wait the pods to be ready. I will include your changes and improve it. Thanks for your support.

Copy link
Owner

@eminaktas eminaktas left a comment

Choose a reason for hiding this comment

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

LGTM!

@eminaktas eminaktas merged commit 3ae2e39 into eminaktas:main Oct 15, 2024
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