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

install/kubernetes: allow extra helm resources #32293

Closed
wants to merge 1 commit into from

Conversation

bewing
Copy link
Contributor

@bewing bewing commented May 1, 2024

Allow additional aribitrary k8s resources to be added to the helm chart via the values file. This allows extension of the existing chart in a way that allows for the extension to be tracked via helm installation history, making rollbacks to earlier state easier than the current case, which involves performing a helm install and then adding/editing additional resources manually.

As an example use-case, you may wish to add additional ClusterRoles and ClusterRoleBindings to the cilium daemonset's service account to let an initContainer or sidecar container watch for LLDP traffic indicating a router, and configure CiliumBgpPeeringPolicies based on that data.

extraResources:
- |
  apiversion: rbac.authorization.k8s.io/v1
  kind: ClusterRole
  metadata:
    name: cilium-bgppeers
  rules:
  - apiGroups:
    - cilium.io
    resources:
    - ciliumbgppeerpolicies
    verbs:
    - get
    - list
    - create
    - patch
- |
  apiVersion: rbac.authorization.k8s.io/v1
  kind: ClusterRoleBinding
  metadata:
    name: cilium-bgppeers
  roleRef:
    apiGroup: rbac.authorization.k8s.io
    kind: ClusterRole
    name: cilium-bgppeers
  subjects:
  - kind: ServiceAccount
    name: cilium
    namespace: kube-system

@bewing bewing requested review from a team as code owners May 1, 2024 21:44
@bewing bewing requested review from youngnick and squeed May 1, 2024 21:44
@maintainer-s-little-helper
Copy link

Commit 7d7153f does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@bewing bewing requested a review from learnitall May 1, 2024 21:44
@maintainer-s-little-helper maintainer-s-little-helper bot added dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels May 1, 2024
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label May 1, 2024
Allow additional aribitrary k8s resources to be added to the helm chart
via the values file.  This allows extension of the existing chart in a
way that allows for the extension to be tracked via helm installation
history, making rollbacks to earlier state easier than the current case,
which involves performing a helm install and then adding/editing
additional resources manually.

As an example use-case, you may wish to add additional ClusterRoles and
ClusterRoleBindings to the cilium daemonset's service account to let an
initContainer or sidecar container watch for LLDP traffic indicating
a router, and configure CiliumBgpPeeringPolicies based on that data.

Signed-off-by: Brandon Ewing <brandon.ewing@imc.com>
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label May 1, 2024
Copy link
Contributor

@learnitall learnitall left a comment

Choose a reason for hiding this comment

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

Hey thanks for the PR! This is an interesting idea, however I'm personally a bit opposed to it because it seems like an anti-pattern. The Cilium helm chart is built to install Cilium in a way that gets it up and running. If there are other resources that a user needs to apply alongside Cilium for their specific use case, and wants to take advantage of Helm's deployment features, it's my opinion that they should create a separate helm chart to do so. For example, a chart could be defined that includes the resources for a specific use-case and then specifies Cilium as a dependency. I'm definitely curious as to what other folks think though.

Copy link

github-actions bot commented Jun 1, 2024

This pull request has been automatically marked as stale because it
has not had recent activity. It will be closed if no further activity
occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Jun 1, 2024
Copy link

This pull request has not seen any activity since it was marked stale.
Closing.

@github-actions github-actions bot closed this Jun 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. kind/community-contribution This was a contribution made by a community member. stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants