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 adr about rke2-flannel #5145

Merged
merged 1 commit into from
Feb 9, 2024
Merged

Conversation

manuelbuil
Copy link
Contributor

@manuelbuil manuelbuil commented Dec 20, 2023

Proposed Changes

ADR. I am proposing a new feature

Types of Changes

New feature proposal

Verification

Testing

Linked Issues

#5215

User-Facing Change


Further Comments

### Weakness
* Yet another cni plugin to support
* Flannel is very limited. We should document it very well to avoid disappointment on non-knowledgeable customers
* How to make it only available for rke2-Windows?
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it even possible to run different CNIs on different nodes? If we support flannel for Windows, my understanding is that we've need to support it for the Linux nodes as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes this is the idea to support flannel as CNI of RKE2 for both linux and windows.

Copy link
Contributor

@brandond brandond Dec 20, 2023

Choose a reason for hiding this comment

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

Should we remove this as a bullet point then, and modify the Proposal section to note that we would support it for all platforms?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah true, I was thinking on not supporting it on "all-linux" nodes. But it does not make sense if we anyway need to support it in linux. I'll remove it


### Weakness
* Yet another cni plugin to support
* Flannel is very limited. We should document it very well to avoid disappointment on non-knowledgeable customers
Copy link
Contributor

@brandond brandond Dec 20, 2023

Choose a reason for hiding this comment

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

Is there a network policy controller available in this configuration, or would clusters configured this way lack netpol support?

Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't support any network policy. The only CNI with network policy and windows support is calico that has the TCP reset issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No network policies. Customers know about this and still they prefer that compared to Calico. Network policies in Windows are apparently not easy to implement

Signed-off-by: Manuel Buil <mbuil@suse.com>
@manuelbuil manuelbuil changed the title Add adr about rke2-win-flannel Add adr about rke2-flannel Dec 21, 2023
Copy link
Contributor

@matttrach matttrach left a comment

Choose a reason for hiding this comment

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

While I agree that this makes a lot of sense, I would like to hear what @briandowns and @rancher-max has to say about it since it would expand our support matrix and possibly change our security assumptions.

Copy link
Contributor

@rancher-max rancher-max left a comment

Choose a reason for hiding this comment

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

I am fine with this. I do agree with you point about documentation -- it should be extremely thorough. I also wonder if we have to use an "rke2-flannel" directly or can we double dip from k3s and pull that in? Or is that what this would effectively be, just packaged as a helm chart instead of embedded?

@brandond
Copy link
Contributor

brandond commented Jan 2, 2024

Ideally it would be a helm chart, same as the other CNIs.

@manuelbuil
Copy link
Contributor Author

I am fine with this. I do agree with you point about documentation -- it should be extremely thorough. I also wonder if we have to use an "rke2-flannel" directly or can we double dip from k3s and pull that in? Or is that what this would effectively be, just packaged as a helm chart instead of embedded?

Helm chart (in linux)! In Windows, I still need to figure it out but probably it will follow Calico's way... AFAIK, Windows does not support pods with access to the root network namespace, which is a requirement for CNI plugin pods. If that is still the case, we can't use helm chart in Windows and, as in Calico, we will need to execute flannel binary (flanneld.exe) from within rke2.exe

@manuelbuil
Copy link
Contributor Author

While I agree that this makes a lot of sense, I would like to hear what @briandowns and @rancher-max has to say about it since it would expand our support matrix and possibly change our security assumptions.

Thanks Matt. What security assumptions? I haven't thought about those

@brandond
Copy link
Contributor

brandond commented Jan 3, 2024

we can't use helm chart in Windows and, as in Calico, we will need to execute flannel binary (flanneld.exe) from within rke2.exe

Yeah, as long as we set it up like Calico, where the Linux nodes use the chart, and the Windows nodes key off the charts presence to run the binary, I think we should be fine.

@manuelbuil
Copy link
Contributor Author

we can't use helm chart in Windows and, as in Calico, we will need to execute flannel binary (flanneld.exe) from within rke2.exe

Yeah, as long as we set it up like Calico, where the Linux nodes use the chart, and the Windows nodes key off the charts presence to run the binary, I think we should be fine.

that's the idea

@matttrach
Copy link
Contributor

I was thinking there is probably some security assumptions in the use of network policies. Without network policies won't pods be basically open for connections?

@manuelbuil
Copy link
Contributor Author

I was thinking there is probably some security assumptions in the use of network policies. Without network policies won't pods be basically open for connections?

Inside the cluster, there will be no limitation for connections to the pods. Connectivity from the outside to the pod is still limited. In any case, we will document that network policies are not available in flannel, users will be aware of this

@manuelbuil
Copy link
Contributor Author

As nobody seems against it, I'll create an Issue and cover all required PRs there

@manuelbuil manuelbuil merged commit 19db21a into rancher:master Feb 9, 2024
2 checks passed
@manuelbuil manuelbuil deleted the flannel-win branch February 9, 2024 13:40
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.

None yet

6 participants