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

cluster: Allow kubevirtci tag overriding #432

Merged
merged 1 commit into from
Jul 4, 2024

Conversation

oshoval
Copy link
Member

@oshoval oshoval commented Jul 3, 2024

What this PR does / why we need it:
It is helpful either manually or for example
when used from CNAO, to use same tag of the cluster that CNAO deployed.
Otherwise a kubevirtci tag mismatch can cause failures once running
KMP e2e tests from CNAO.

Special notes for your reviewer:

Release note:

None

@kubevirt-bot
Copy link
Collaborator

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

@oshoval oshoval changed the title WIP WIP Allow overriding kubevirtci tag Jul 3, 2024
@oshoval oshoval changed the title WIP Allow overriding kubevirtci tag cluster: Allow kubevirtci tag overriding Jul 3, 2024
@oshoval oshoval marked this pull request as ready for review July 3, 2024 12:55
@kubevirt-bot kubevirt-bot requested a review from ormergi July 3, 2024 12:55
@oshoval
Copy link
Member Author

oshoval commented Jul 3, 2024

/cc @RamLavi
we need this (+ release once it is merged please)
so we can bump kubevirtci on CNAO
otherwise if the cluster kubevirtci tag CNAO deployed is different then the kubevirtci helper scripts that KMP uses
it can product failures.
with this small change, the tag CNAO uses will be also used whenever needed when CNAO runs KMP e2e tests
(which is the correct thing to do, regardless of bumping)

We need this fix on all repos, so far did on OVS and KMP, where i already so problems where it was missing.

@kubevirt-bot kubevirt-bot requested a review from RamLavi July 3, 2024 12:58
@oshoval
Copy link
Member Author

oshoval commented Jul 3, 2024

/test pull-kubemacpool-e2e-k8s

@oshoval
Copy link
Member Author

oshoval commented Jul 3, 2024

need first to adapt bump-kubevirtci.sh

@oshoval oshoval force-pushed the kci branch 2 times, most recently from 8a89630 to 8be74f3 Compare July 4, 2024 06:09
It is helpful either manually or for example
when used from CNAO, to use same tag of the cluster that CNAO deployed.
Otherwise a kubevirtci tag mismatch can cause failures once running
KMP e2e tests from CNAO.

Signed-off-by: Or Shoval <[email protected]>
Copy link
Member

@qinqon qinqon left a comment

Choose a reason for hiding this comment

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

/lgtm
/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 merged commit 98d8b82 into k8snetworkplumbingwg:main Jul 4, 2024
5 checks passed
@oshoval
Copy link
Member Author

oshoval commented Jul 4, 2024

Thanks
Can you please create a patch release 0.43.1 on main so CNAO consume it ?
@phoracek @RamLavi @qinqon

It will be indeed nice when we implement mechanism to allow consuming non core related changes
by CNAO, i will work on it when i have time.

@RamLavi
Copy link
Member

RamLavi commented Jul 4, 2024

@oshoval you need to backport this to release-0.43 first no?

@RamLavi
Copy link
Member

RamLavi commented Jul 4, 2024

@oshoval you need to backport this to release-0.43 first no?

oh my bad, we are already after CF on that stable branch.
This change will only affect U/S, am I correct?
And only the U/S dev env (i.e. the lanes etc..) right?
If the answer is yes to both of the above then I assume that backporting this change is safe enough to do without getting QE to approve it and all that, but I still wonder why you would want to backport it to that stable branch - It's going GA in a few days

Perhaps you meant you want to issue a new main release (which will be minor 0.44.0 IIUC)? - in that case I'm all forward

@oshoval
Copy link
Member Author

oshoval commented Jul 4, 2024

@oshoval you need to backport this to release-0.43 first no?

oh my bad, we are already after CF on that stable branch. This change will only affect U/S, am I correct? And only the U/S dev env (i.e. the lanes etc..) right? If the answer is yes to both of the above then I assume that backporting this change is safe enough to do without getting QE to approve it and all that, but I still wonder why you would want to backport it to that stable branch - It's going GA in a few days

Perhaps you meant you want to issue a new main release (which will be minor 0.44.0 IIUC)? - in that case I'm all forward

this change doesn't affect KMP at all (as it only allow external overriding the default value)
we just need a main release indeed so CNAO can consume it (it will help CNAO, not KMP)
ye we can use 0.44.0 if 0.43.0 is stable, in any case since CNAO main watches KMP main, the release must be on main
we just need CNAO to consume it, didn't mean i want to backport it, just looked what is the last release naively

can you please create new minor 0.44.0 on main ?

Thanks

@RamLavi
Copy link
Member

RamLavi commented Jul 4, 2024

can you please create new minor 0.44.0 on main ?

https://github.com/k8snetworkplumbingwg/kubemacpool/releases/tag/v0.44.0

@oshoval
Copy link
Member Author

oshoval commented Jul 4, 2024

can you please create new minor 0.44.0 on main ?

https://github.com/k8snetworkplumbingwg/kubemacpool/releases/tag/v0.44.0

Thanks !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants