Add flannel resource as optional CNI #3853

praveenkumar · 2024-09-03T10:20:58Z

As of now this is going to draft mode and I will rebase it time to time and get feedback around it.

Right now kube-proxy and flannel resource added to assets/optional so that it will not increase the core microshift binary size and also have spec file change so that flannel can be optional package and it kube-proxy resources also added to this package

It now include network.cniPlugin as config option and flannel package put a drop in config file which have this value as "none".

Steps to test :

Have a update RHEL VM/machine
Get this PR and do make rpm
Install required RPMS (for me)

sudo dnf install _output/rpmbuild/RPMS/x86_64/microshift-networking-4.18.0_0.nightly_2024_08_29_020346_20240904075132_bc6910c6f_dirty-1.el9.x86_64.rpm \
   _output/rpmbuild/RPMS/x86_64/microshift-4.18.0_0.nightly_2024_08_29_020346_20240904075132_bc6910c6f_dirty-1.el9.x86_64.rpm \
   _output/rpmbuild/RPMS/noarch/microshift-greenboot-4.18.0_0.nightly_2024_08_29_020346_20240904075132_bc6910c6f_dirty-1.el9.noarch.rpm \
   _output/rpmbuild/RPMS/noarch/microshift-selinux-4.18.0_0.nightly_2024_08_29_020346_20240904075132_bc6910c6f_dirty-1.el9.noarch.rpm \
   _output/rpmbuild/RPMS/x86_64/microshift-kube-proxy-4.18.0_0.nightly_2024_08_29_020346_20240904075132_bc6910c6f_dirty-1.el9.x86_64.rpm \
   _output/rpmbuild/RPMS/x86_64/microshift-flannel-4.18.0_0.nightly_2024_08_29_020346_20240904075132_bc6910c6f_dirty-1.el9.x86_64.rpm

start the microshift service and check the pods

$ sudo oc get pods -A --kubeconfig /var/lib/microshift/resources/kubeadmin/kubeconfig
NAMESPACE              NAME                                       READY   STATUS                 RESTARTS   AGE
kube-flannel           kube-flannel-ds-k7tfc                      1/1     Running                0          59s
kube-proxy             kube-proxy-5m752                           1/1     Running                0          59s
kube-system            csi-snapshot-controller-5795dc77b6-qxltx   1/1     Running                0          57s
kube-system            csi-snapshot-webhook-859bc7dcdf-mh86b      1/1     Running                0          61s
openshift-dns          dns-default-l7dgw                          2/2     Running                0          47s
openshift-dns          node-resolver-6s254                        1/1     Running                0          59s
openshift-ingress      router-default-74c8bbb85b-wkrht            1/1     Running                0          56s
openshift-service-ca   service-ca-78fbd74695-fzdgj                1/1     Running                0          56s

openshift-ci · 2024-09-03T10:21:01Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

openshift-ci · 2024-09-03T10:21:13Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: praveenkumar
Once this PR has been reviewed and has the lgtm label, please assign pacevedom for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

praveenkumar · 2024-09-03T10:21:42Z

assets/optional/flannel/release-flannel-x86_64.json

@@ -0,0 +1,9 @@
+{
+ "release": {
+ "base": ""


Since flannel image is not associated with OCP release what would be the base for this image?

IMO, let's still have the OCP version here, like we're doing for OLM and Multus.
In any case, this should be automatically updated during rebase.

assets/optional/flannel/00-namespace.yaml

ggiguash · 2024-09-04T11:09:51Z

assets/optional/flannel/release-flannel-x86_64.json

@@ -0,0 +1,9 @@
+{
+ "release": {
+ "base": ""


IMO, let's still have the OCP version here, like we're doing for OLM and Multus.
In any case, this should be automatically updated during rebase.

ggiguash · 2024-09-05T09:11:49Z

packaging/microshift/config.yaml

@@ -133,4 +133,5 @@ storage:
 # Allowed values are: unset, [], or one or more of ["snapshot-controller", "snapshot-webhook"]
 optionalCsiComponents:
 - ""
-
+# Disable default CNI so user can use different CNI than OVN-K
+DisableDefaultCNI: false


Should we put it under the network section?
Also, please change the name to disableDefaultCNI to be compatible with others.

Yes, I think better to have it under network section because if it is disabled then any config on network section is not going to work. Also should we put the config name as cniPlugin and use same like storage "" , "ovnk", and `"none" which will disable it?

Yes, I like the consistency!

so the default will remain, if the value is unset it will defer to "ovnk" right?

@eslutsky right, same as what we have for storage. I have updated this now.

This option is used to disable default OVN-K plugin and default value for this would be empty string. Also add warning in case this it is disabled.

openshift-ci · 2024-09-05T17:57:08Z

@praveenkumar: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/microshift-metal-tests-arm	`158c99d`	link	true	`/test microshift-metal-tests-arm`
ci/prow/test-rebase	`158c99d`	link	false	`/test test-rebase`
ci/prow/microshift-metal-tests	`158c99d`	link	true	`/test microshift-metal-tests`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

eslutsky

after disabling the CNI, microshift.service will still be dependent on : openvswitch.service and microshift-ovs-init.service

see: https://github.com/openshift/microshift/blob/main/packaging/systemd/microshift.service#L4

ggiguash · 2024-09-06T05:49:38Z

after disabling the CNI, microshift.service will still be dependent on : openvswitch.service and microshift-ovs-init.service

see: https://github.com/openshift/microshift/blob/main/packaging/systemd/microshift.service#L4

That's a great catch, @eslutsky !
One way to solve this is for microshift-flannel RPM to install a systemd drop-in for microshift.service that overrides the service dependency.

praveenkumar · 2024-09-06T06:58:55Z

after disabling the CNI, microshift.service will still be dependent on : openvswitch.service and microshift-ovs-init.service
see: https://github.com/openshift/microshift/blob/main/packaging/systemd/microshift.service#L4

That's a great catch, @eslutsky ! One way to solve this is for microshift-flannel RPM to install a systemd drop-in for microshift.service that overrides the service dependency.

As per man page of systemd-unit (https://man.archlinux.org/man/systemd.unit.5.en) looks like we can't remove the After and Wants declarative as part of drop in unit file. I think microshift-network package should have the drop in file to add openvswitch.service and microshift-ovs-init.service service as part of drop-in files because this package should require that?

Dependencies (After=, etc.) cannot be reset to an empty list, so dependencies can only be added in drop-ins. If you want to remove dependencies, you have to override the entire unit.

ggiguash · 2024-09-06T07:12:07Z

@praveenkumar , let's override the full list in microshift-flannel RPM drop-in. It should be less disruptive.

Wants=network-online.target crio.service
After=network-online.target crio.service

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 3, 2024

praveenkumar commented Sep 3, 2024

View reviewed changes

praveenkumar force-pushed the flannel branch from 95768ef to bc6910c Compare September 4, 2024 08:35

ggiguash reviewed Sep 4, 2024

View reviewed changes

praveenkumar force-pushed the flannel branch from bc6910c to 3ea165d Compare September 5, 2024 08:54

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 5, 2024

praveenkumar force-pushed the flannel branch from 3ea165d to f0085b7 Compare September 5, 2024 08:56

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 5, 2024

praveenkumar force-pushed the flannel branch from f0085b7 to 292d6d6 Compare September 5, 2024 09:06

ggiguash reviewed Sep 5, 2024

View reviewed changes

praveenkumar force-pushed the flannel branch from 292d6d6 to e83b1df Compare September 5, 2024 11:52

praveenkumar marked this pull request as ready for review September 5, 2024 12:13

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 5, 2024

openshift-ci bot requested review from copejon and pacevedom September 5, 2024 12:15

praveenkumar added 2 commits September 5, 2024 21:34

Add flannel resource to optional assets

482f10d

Add kube-proxy resource to assets/optional

c5c321b

praveenkumar force-pushed the flannel branch from e83b1df to 86813e9 Compare September 5, 2024 16:08

praveenkumar added 2 commits September 5, 2024 21:54

Add cniPlugin option in network config

0a6c478

This option is used to disable default OVN-K plugin and default value for this would be empty string. Also add warning in case this it is disabled.

microshift.spec: Add flannel as subpackage for microshift

158c99d

praveenkumar force-pushed the flannel branch from 86813e9 to 158c99d Compare September 5, 2024 16:24

eslutsky reviewed Sep 5, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add flannel resource as optional CNI #3853

Add flannel resource as optional CNI #3853

praveenkumar commented Sep 3, 2024 •

edited

Loading

openshift-ci bot commented Sep 3, 2024

openshift-ci bot commented Sep 3, 2024

praveenkumar Sep 3, 2024 •

edited

Loading

ggiguash Sep 4, 2024

ggiguash Sep 4, 2024

ggiguash Sep 5, 2024

praveenkumar Sep 5, 2024

ggiguash Sep 5, 2024

eslutsky Sep 5, 2024

praveenkumar Sep 5, 2024

openshift-ci bot commented Sep 5, 2024

eslutsky left a comment

ggiguash commented Sep 6, 2024

praveenkumar commented Sep 6, 2024

ggiguash commented Sep 6, 2024

Add flannel resource as optional CNI #3853

Are you sure you want to change the base?

Add flannel resource as optional CNI #3853

Conversation

praveenkumar commented Sep 3, 2024 • edited Loading

openshift-ci bot commented Sep 3, 2024

openshift-ci bot commented Sep 3, 2024

praveenkumar Sep 3, 2024 • edited Loading

Choose a reason for hiding this comment

ggiguash Sep 4, 2024

Choose a reason for hiding this comment

ggiguash Sep 4, 2024

Choose a reason for hiding this comment

ggiguash Sep 5, 2024

Choose a reason for hiding this comment

praveenkumar Sep 5, 2024

Choose a reason for hiding this comment

ggiguash Sep 5, 2024

Choose a reason for hiding this comment

eslutsky Sep 5, 2024

Choose a reason for hiding this comment

praveenkumar Sep 5, 2024

Choose a reason for hiding this comment

openshift-ci bot commented Sep 5, 2024

eslutsky left a comment

Choose a reason for hiding this comment

ggiguash commented Sep 6, 2024

praveenkumar commented Sep 6, 2024

ggiguash commented Sep 6, 2024

praveenkumar commented Sep 3, 2024 •

edited

Loading

praveenkumar Sep 3, 2024 •

edited

Loading