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

Bump ipam to v0.1.9-alpha and enable managed tap #1954

Merged
merged 4 commits into from
Dec 18, 2024

Conversation

RamLavi
Copy link
Collaborator

@RamLavi RamLavi commented Dec 17, 2024

What this PR does / why we need it:
Bump kubevirt-ipam-controller to v0.1.9-alpha
Adapt to support IPAM UDN tests with managedTap binding on CNAO.

Special notes for your reviewer:

Release note:

NONE

@kubevirt-bot kubevirt-bot added release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Dec 17, 2024
@@ -1,6 +1,19 @@
#!/bin/bash
set -exo pipefail

if [ -z "${OCI_BIN}" ];then
Copy link
Collaborator

@oshoval oshoval Dec 17, 2024

Choose a reason for hiding this comment

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

this is weird, we concluded at the past that we are doing it once on the bootstrap / locally and not here,
if it is not mandatory please remove it (i mean line 9-11)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since we are deploying kubevirt manually at CNAO then we don't do it.

We can alternatively remove the use of this script and use the -kv flag at OVNK.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

IMO deploying kubevirt is part of cluster uping on the lane, so it's totally OK to add this

Copy link
Collaborator

@oshoval oshoval Dec 17, 2024

Choose a reason for hiding this comment

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

we can't add the -kv, because it installs ipam and we dont want to reinstall it
and also want to control what kubevirt version we install
i would say please add it (sysfs) to the git actions once instead, no need each time
(assuming that for local deployment, it can also be done one time on the bare metal)
this what also was decided fwiw when we had same discussions on other repos

Copy link
Collaborator

Choose a reason for hiding this comment

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

a follow-up can be to adapt OVN to be robust with flags that we can use here, as long as it is nice for both projects

Copy link
Collaborator

Choose a reason for hiding this comment

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

moreover it is not required for kubevirtci ones, and dont need to be done always, this is another reason it is better please to have it only on the git actions

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moved it to the automation script so it only involves ipam-ext lane.

@RamLavi RamLavi force-pushed the bump_ipam_with_managedTap branch from 9b5a028 to 9f98bb8 Compare December 17, 2024 15:13
RamLavi and others added 4 commits December 17, 2024 17:14
In order to prevent virt-handler crashing on kind cluster on ipam-ext CI,
increasing max files on cluster nodes

Signed-off-by: Ram Lavi <[email protected]>
Adding this kind flag is need in order to enable the primary UDN feature
on OVNK

Signed-off-by: Ram Lavi <[email protected]>
@RamLavi RamLavi force-pushed the bump_ipam_with_managedTap branch from 9f98bb8 to 8bf6ddc Compare December 17, 2024 15:14
make cluster-up

trap teardown EXIT

increase_ulimit
Copy link
Collaborator

Choose a reason for hiding this comment

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

much better thx

@RamLavi
Copy link
Collaborator Author

RamLavi commented Dec 17, 2024

@qinqon PTAL

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Dec 18, 2024
@qinqon
Copy link
Collaborator

qinqon commented Dec 18, 2024

/approve

@kubevirt-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qinqon

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

The pull request process is described here

Needs approval from an approver in each of these files:

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

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 18, 2024
@kubevirt-bot kubevirt-bot merged commit 02a9c81 into kubevirt:main Dec 18, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants