USHIFT-4342: Add flannel resource as optional CNI by praveenkumar · Pull Request #3853 · openshift/microshift

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

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

praveenkumar · 2024-09-10T02:56:03Z

@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

@ggiguash So I tried following but that doesn't override the dependencies and openvswitch.service microshift-ovs-init.service services are still starting :( So the doc says to override the dependencies, we need to rewrite the unit file.

# cat /etc/systemd/system/microshift.service.d/flannel.conf
[Unit]
Description=MicroShift
Wants=network-online.target crio.service
After=network-online.target crio.service

And the systemd unit file looks like

$ systemctl cat microshift
# /usr/lib/systemd/system/microshift.service
[Unit]
Description=MicroShift
Wants=network-online.target crio.service openvswitch.service microshift-ovs-init.service
After=network-online.target crio.service openvswitch.service microshift-ovs-init.service

# Control shutdown order by declaring this service to start Before the kubepods.slice
# transient systemd unit; this makes system shutdown delay MicroShift shutdown until
# all the pod containers are down. This is important because some services need to talk
# to the MicroShift API during shutdown (i.e. releasing leader election locks or cleaning
# up other resources) MicroShift restart or manual stop will not stop the kubepods.
Before=kubepods.slice

[Service]
WorkingDirectory=/usr/bin/
ExecStart=microshift run
Restart=always
User=root
Type=notify
Delegate=yes
CPUAccounting=yes
BlockIOAccounting=yes
MemoryAccounting=yes
LimitNOFILE=1048576
TimeoutStartSec=4m

[Install]
WantedBy=multi-user.target
Also=microshift-cleanup-kubelet.service

# /etc/systemd/system/microshift.service.d/flannel.conf
[Unit]
Description=MicroShift
Wants=network-online.target crio.service
After=network-online.target crio.service

But when I start the microshift service I can see microshift-ovs-init.service still run.

$ systemctl status microshift-ovs-init.service
○ microshift-ovs-init.service - Configures Open vSwitch for OVN (MicroShift)
     Loaded: loaded (/usr/lib/systemd/system/microshift-ovs-init.service; static)
     Active: inactive (dead)

$ sudo systemctl start microshift

$ systemctl status microshift-ovs-init.service
○ microshift-ovs-init.service - Configures Open vSwitch for OVN (MicroShift)
     Loaded: loaded (/usr/lib/systemd/system/microshift-ovs-init.service; static)
     Active: inactive (dead) since Tue 2024-09-10 02:53:49 UTC; 1min 15s ago
    Process: 623 ExecStart=/bin/bash /usr/bin/configure-ovs.sh (code=exited, status=0/SUCCESS)
    Process: 643 ExecStart=/bin/bash /usr/bin/configure-ovs-microshift.sh (code=exited, status=0/SUCCESS)
   Main PID: 643 (code=exited, status=0/SUCCESS)
        CPU: 27ms

Sep 10 02:53:49 ec8e3cdfe288 bash[643]: + OVS_HANDLER_THREADS=1
Sep 10 02:53:49 ec8e3cdfe288 bash[643]: + OVS_REVALIDATOR_THREADS=1
Sep 10 02:53:49 ec8e3cdfe288 bash[643]: + '[' 1 '!=' auto ']'
Sep 10 02:53:49 ec8e3cdfe288 bash[643]: + ovs-vsctl set open . other_config:n-handler-threads=1
Sep 10 02:53:49 ec8e3cdfe288 ovs-vsctl[644]: ovs|00001|vsctl|INFO|Called as ovs-vsctl set open . other_config:n-handler-threads=1
Sep 10 02:53:49 ec8e3cdfe288 bash[643]: + '[' 1 '!=' auto ']'
Sep 10 02:53:49 ec8e3cdfe288 bash[643]: + ovs-vsctl set open . other_config:n-revalidator-threads=1
Sep 10 02:53:49 ec8e3cdfe288 ovs-vsctl[646]: ovs|00001|vsctl|INFO|Called as ovs-vsctl set open . other_config:n-revalidator-threads=1
Sep 10 02:53:49 ec8e3cdfe288 systemd[1]: microshift-ovs-init.service: Deactivated successfully.
Sep 10 02:53:49 ec8e3cdfe288 systemd[1]: Finished Configures Open vSwitch for OVN (MicroShift).

ggiguash · 2024-09-10T15:13:44Z

@praveenkumar , looking at man systemd.unit, it has the following text:

       Note that for drop-in files, if one wants to remove entries from a setting that is
       parsed as a list (and is not a dependency), such as AssertPathExists= (or e.g.
       ExecStart= in service units), one needs to first clear the list before re-adding all
       entries except the one that is to be removed. 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.

So, it seems that we have no choice but to create a copy of microshift/packaging/systemd/microshift.service (i.e. microshift-flannel.service) and install it at /etc/systemd/system/microshift.service to override the default service settings.

The following seems to work correctly to as a proof of concept:

# cp /usr/lib/systemd/system/microshift.service /etc/systemd/system
# vi /etc/systemd/system/microshift.service
# systemctl daemon-reload 

# systemctl cat microshift | head -5
# /etc/systemd/system/microshift.service
[Unit]
Description=MicroShift
Wants=network-online.target crio.service
After=network-online.target crio.service

praveenkumar · 2024-09-10T16:06:16Z

@praveenkumar , looking at man systemd.unit, it has the following text:

       Note that for drop-in files, if one wants to remove entries from a setting that is
       parsed as a list (and is not a dependency), such as AssertPathExists= (or e.g.
       ExecStart= in service units), one needs to first clear the list before re-adding all
       entries except the one that is to be removed. 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.

Exactly, this is what I mention in #3853 (comment) as part of man page snip :)

So, it seems that we have no choice but to create a copy of microshift/packaging/systemd/microshift.service (i.e. microshift-flannel.service) and install it at /etc/systemd/system/microshift.service to override the default service settings.

Right, this is the way to achieve it. Now should we have this copy in packaging/flannel/microshift.service or we use some sed magic in the spec file to remove this deps ?

The following seems to work correctly to as a proof of concept:

# cp /usr/lib/systemd/system/microshift.service /etc/systemd/system
# vi /etc/systemd/system/microshift.service
# systemctl daemon-reload 

# systemctl cat microshift | head -5
# /etc/systemd/system/microshift.service
[Unit]
Description=MicroShift
Wants=network-online.target crio.service
After=network-online.target crio.service

ggiguash · 2024-09-10T16:17:22Z

@praveenkumar , let's have a copy of the unit at packaging/systemd/microshift-flannel.service and have it renamed to /etc/systemd/system/microshift.service during the installation.

This way, I hope, it will be obvious that changes in one unit file would need to be applied on the other.

praveenkumar · 2024-09-11T03:19:07Z

@praveenkumar , let's have a copy of the unit at packaging/systemd/microshift-flannel.service and have it renamed to /etc/systemd/system/microshift.service during the installation.

This way, I hope, it will be obvious that changes in one unit file would need to be applied on the other.

Sure, Updated it accordingly.

ggiguash · 2024-09-13T08:40:03Z

/retitle USHIFT-4342: Add flannel resource as optional CNI

openshift-ci-robot · 2024-09-13T08:40:37Z

@praveenkumar: This pull request references USHIFT-4342 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

Details

In response to this:

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

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 openshift-eng/jira-lifecycle-plugin repository.

praveenkumar · 2024-09-16T05:10:38Z

/retest

praveenkumar · 2024-09-17T07:12:55Z

/retest

praveenkumar · 2024-09-17T11:27:37Z

/test test-rebase

praveenkumar · 2024-09-18T11:50:24Z

/test ocp-conformance-rhel-eus-arm

praveenkumar · 2024-09-19T06:18:01Z

/test e2e-aws-footprint-and-performance

praveenkumar · 2024-09-20T08:53:23Z

/test e2e-aws-footprint-and-performance

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.

`WITH_FLANNEL` env now we can build the flannel package with condition and default value is `0` which means not building it.

openshift-ci · 2024-09-23T12:03:04Z

@praveenkumar: The following test 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/e2e-aws-footprint-and-performance	7d0c160e82ad90976bd12a543c19686c7fe1c819	link	true	`/test e2e-aws-footprint-and-performance`

Full PR test history. Your PR dashboard.

Details

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.

praveenkumar · 2024-09-23T12:29:08Z

/retest

eslutsky · 2024-09-26T09:58:49Z

/lgtm

openshift-ci · 2024-09-26T09:59:11Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: eslutsky, praveenkumar

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [eslutsky]

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

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

Comment thread assets/optional/flannel/release-flannel-x86_64.json Outdated

ggiguash reviewed Sep 4, 2024

View reviewed changes

Comment thread assets/optional/flannel/00-namespace.yaml Outdated

Comment thread assets/optional/flannel/release-flannel-x86_64.json Outdated

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

ggiguash reviewed Sep 5, 2024

View reviewed changes

Comment thread packaging/microshift/config.yaml Outdated

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

eslutsky reviewed Sep 5, 2024

View reviewed changes

eslutsky mentioned this pull request Sep 9, 2024

USHIFT-4335 USHIFT-4354 USHIFT-4356: build runnable microshift-okd bootc container #3830

Merged

ggiguash requested changes Sep 11, 2024

View reviewed changes

Comment thread packaging/rpm/microshift.spec Outdated

Comment thread packaging/rpm/microshift.spec Outdated

Comment thread packaging/rpm/microshift.spec Outdated

Comment thread packaging/rpm/microshift.spec Outdated

openshift-merge-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Sep 11, 2024

openshift-ci Bot changed the title ~~Add flannel resource as optional CNI~~ USHIFT-4342: Add flannel resource as optional CNI Sep 13, 2024

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Sep 13, 2024

openshift-merge-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Sep 13, 2024

ggiguash reviewed Sep 23, 2024

View reviewed changes

Comment thread packaging/rpm/microshift.spec Outdated

praveenkumar added 5 commits September 23, 2024 15:34

Add flannel resource to optional assets

4d3029f

Add kube-proxy resource to assets/optional

e20f60b

Add cniPlugin option in network config

db89c73

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

954833d

Make flannel package optional to build

53ed9fb

`WITH_FLANNEL` env now we can build the flannel package with condition and default value is `0` which means not building it.

openshift-ci Bot assigned eslutsky Sep 26, 2024

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Sep 26, 2024

openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 26, 2024

openshift-merge-bot Bot merged commit 305476e into openshift:main Sep 26, 2024

Conversation

praveenkumar commented Sep 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci Bot commented Sep 3, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eslutsky left a comment

Choose a reason for hiding this comment

Uh oh!

ggiguash commented Sep 6, 2024

Uh oh!

praveenkumar commented Sep 6, 2024

Uh oh!

ggiguash commented Sep 6, 2024

Uh oh!

praveenkumar commented Sep 10, 2024

Uh oh!

ggiguash commented Sep 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

praveenkumar commented Sep 10, 2024

Uh oh!

ggiguash commented Sep 10, 2024

Uh oh!

praveenkumar commented Sep 11, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ggiguash commented Sep 13, 2024 • edited by openshift-ci Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Sep 13, 2024 • edited by openshift-ci Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

praveenkumar commented Sep 16, 2024

Uh oh!

praveenkumar commented Sep 17, 2024

Uh oh!

praveenkumar commented Sep 17, 2024

Uh oh!

praveenkumar commented Sep 18, 2024

Uh oh!

praveenkumar commented Sep 19, 2024

Uh oh!

praveenkumar commented Sep 20, 2024

Uh oh!

Uh oh!

openshift-ci Bot commented Sep 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

praveenkumar commented Sep 23, 2024

Uh oh!

eslutsky commented Sep 26, 2024

Uh oh!

openshift-ci Bot commented Sep 26, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

praveenkumar commented Sep 3, 2024 •

edited

Loading

ggiguash commented Sep 10, 2024 •

edited

Loading

ggiguash commented Sep 13, 2024 •

edited by openshift-ci Bot

Loading

openshift-ci-robot commented Sep 13, 2024 •

edited by openshift-ci Bot

Loading

openshift-ci Bot commented Sep 23, 2024 •

edited

Loading