Add k8s-daemonset helm-chart - inital cut

[rkatti]

Description

Added initial helm-chart for k8s-daemonset.

Documentation

TODOs

Please ensure all of these TODOs are completed before asking for a review.

• Ensure the branch is named correctly with the issue number. e.g: feature/new-vpc-endpoints-955 or bug/missing-count-param-434.
• Update the docs.
• Keep the changes backward compatible where possible.
• Run the pre-commit checks successfully.
• Run the relevant tests successfully.
• Ensure any 3rd party code adheres with our license policy or delete this line if its not applicable.

Related Issues

#23

[yorinasub17]

Looks like a good start! I understand this is still WIP, so I focused on the bigger picture in my review. Here are a few high level issues I spotted in the current bundle:

• AFAIK, Service and Ingress are atypical resources for DaemonSet. I think we can omit them, at least in the initial implementation.

  • Typically DaemonSet are used for deploying node specific managers. What this means is that in most use cases, it is important for Pods on the same nodes to stream to the instance of the DaemonSet on that node. For example, the nodelocal-dns cache is deployed as a DaemonSet, and all pods on the node want to talk to the instance on the same node.
  • On the flip side, Service and Ingress provide a common routing endpoint for accessing any Pod in the target set. So using a Service or Ingress endpoint to access a DaemonSet means you could end up routing to another instance of the Pod on a different Node.

• You can also omit ServiceMonitor and google ManagedCertificate, since they relate to Service and Ingress, so once those are removed, these are moot.

• Similarly, PodDisruptionBudget doesn't quite make sense for a DaemonSet either, and can be removed.

  • PodDisruptionBudget is used to ensure a minimum number of Pods for the app is up at a time during voluntary disruption (e.g., rotating worker nodes). This concept doesn't make sense for a DaemonSet, because there is typically one replica of the Pod per node, leading to a situation where the disruption budget is really a budget of the number of nodes that can be up at a time. But Kubernetes can't manage the node disruptions - it can only manage Pods, so the scope is incorrect.

NOTE: I focused on the chart code and ignored the README in this initial review. It looks like the README is still a WIP, so will hold off on going through it until you've got a chance to groom it a little.

[yorinasub17]

These appear to be specific to the fluentd example, and should instead be managed in the values.yaml for that example instead of hardcoded here.

[rkatti]

@yorinasub17 @rhoboat - I've just refactored and simplified the chart based on your feedback. Also, README.md has been updated with the needful. Also, I've tested the example chart with minikube. Please let me know if you've any further inputs.

[rhoboat]

It's a good start! I was going to leave very similar comments to Yori, so I will refrain from restating things.