-
Notifications
You must be signed in to change notification settings - Fork 48
Conversation
Signed-off-by: Imran Pochi <[email protected]>
This commit adds calico-kube-controllers ServiceAccount, ClusterRoleBinding and ClusterRole to the correct location `cluster-role-binding.yaml`, `cluster-role.yaml` and `service-account.yaml`. Signed-off-by: Imran Pochi <[email protected]>
This commit updates Calico to the version 3.16.4. Renames the file `daemonset.yaml` to correctly reflect the resource it contains i.e `calico-node.yaml` Renames the file `deployment.yaml` to correctly reflect the resource it contains i.e `calico-kube-controllers.yaml` Updates the calico config. Signed-off-by: Imran Pochi <[email protected]>
2472d1f
to
b94e31f
Compare
@@ -0,0 +1,50 @@ | |||
# See https://github.com/projectcalico/kube-controllers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this change on purpose?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean moving from deployment.yaml to this file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No I mean , this makes the file naming correct to the resource it holds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the file was named deployment.yaml
, it contain the deployment resource. I renamed it reflect better.
Similar with daemonset.yaml
deployment.yaml - calico-kube-controllers.yaml
daemonset.yaml - calico-node.yaml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some nits, otherwise LGTM.
@@ -3,7 +3,7 @@ kind: CustomResourceDefinition | |||
metadata: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
- calico: update calico CRDS
+ calico: update calico CRDs
@@ -1,3 +1,4 @@ | |||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly, the best would be to have everything in calico.yaml
, as from wget https://docs.projectcalico.org/v3.16.4/manifests/calico.yaml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, we should look into integrating Tigera Operator in our helm chart.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, perhaps this is the way to go.
Created #1125 for tracking.
This PR updates calico to 3.16.4