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

kubevirtci: Bump version #1807

Merged
merged 2 commits into from
Jul 4, 2024
Merged

kubevirtci: Bump version #1807

merged 2 commits into from
Jul 4, 2024

Conversation

oshoval
Copy link
Collaborator

@oshoval oshoval commented Jun 9, 2024

What this PR does / why we need it:
Bump kubevirtci and cleanup unrequired exports.

Allow pinning specific kubevirtci per lane.
It will allow decoupling kubevirtci tag version between the lanes.

Special notes for your reviewer:

Release note:

None

@kubevirt-bot kubevirt-bot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jun 9, 2024
Copy link

sonarcloud bot commented Jun 9, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@oshoval
Copy link
Collaborator Author

oshoval commented Jun 10, 2024

closing
we will try to use github actions

this need to be fixed on a follow-up orthogonal effort

note - one of them happens also on clean main
#1809 (comment)

@oshoval oshoval closed this Jun 10, 2024
@oshoval oshoval reopened this Jul 3, 2024
@oshoval oshoval marked this pull request as draft July 3, 2024 06:49
@kubevirt-bot kubevirt-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 3, 2024
@oshoval oshoval marked this pull request as ready for review July 3, 2024 06:52
@kubevirt-bot kubevirt-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 3, 2024
@oshoval
Copy link
Collaborator Author

oshoval commented Jul 3, 2024

@phoracek
ptal, as we spoke offline

@@ -18,7 +18,7 @@ versionChanged() {
}

main() {
export KUBEVIRT_PROVIDER='k8s-1.25'
export KUBEVIRT_PROVIDER='k8s-1.28'
Copy link
Collaborator Author

@oshoval oshoval Jul 3, 2024

Choose a reason for hiding this comment

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

we can even drop this line (and the one in the next 2 files)
as it is the default anyhow taken from cluster.sh

Copy link
Member

Choose a reason for hiding this comment

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

Please drop it

Copy link
Collaborator Author

@oshoval oshoval Jul 3, 2024

Choose a reason for hiding this comment

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

done

@kubevirt-bot kubevirt-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 3, 2024
@oshoval oshoval marked this pull request as draft July 3, 2024 09:04
@kubevirt-bot kubevirt-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 3, 2024
@oshoval
Copy link
Collaborator Author

oshoval commented Jul 3, 2024

/test pull-e2e-cluster-network-addons-operator-kubemacpool-functests

@oshoval
Copy link
Collaborator Author

oshoval commented Jul 3, 2024

/test pull-e2e-cluster-network-addons-operator-kubemacpool-functests

@oshoval
Copy link
Collaborator Author

oshoval commented Jul 3, 2024

passed
lets retry

/test pull-e2e-cluster-network-addons-operator-kubemacpool-functests

edit passed twice

@@ -20,6 +20,10 @@ main() {
source automation/check-patch.setup.sh
cd ${TMP_PROJECT_PATH}

# until we fix OVS lane on the newer kubevirtci
Copy link
Member

Choose a reason for hiding this comment

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

Mark this with TODO and link the tracker of the issue

Copy link
Collaborator Author

@oshoval oshoval Jul 3, 2024

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

@oshoval oshoval Jul 3, 2024

Choose a reason for hiding this comment

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

dropped since the root cause is fixed

@@ -18,7 +18,7 @@ versionChanged() {
}

main() {
export KUBEVIRT_PROVIDER='k8s-1.25'
export KUBEVIRT_PROVIDER='k8s-1.28'
Copy link
Member

Choose a reason for hiding this comment

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

Please drop it

@@ -99,6 +99,15 @@ git-utils::fetch_component ${component_path} ${component_url} ${component_commit

export TMP_COMPONENT_PATH=${component_path}

# TODO remove
Copy link
Member

Choose a reason for hiding this comment

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

Either remove this or provide enough context to make this actionable

Copy link
Collaborator Author

@oshoval oshoval Jul 3, 2024

Choose a reason for hiding this comment

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

gonna be dropped before this PR is merged
just used for poc

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

removed, lets see all pass (required PRs were merged on OVS/KMP)

@@ -99,6 +99,15 @@ git-utils::fetch_component ${component_path} ${component_url} ${component_commit

export TMP_COMPONENT_PATH=${component_path}

# TODO remove
if [[ COMPONENT=="kubemacpool" ]]; then
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if [[ COMPONENT=="kubemacpool" ]]; then
if [[ $COMPONENT == "kubemacpool" ]]; then

IIUIC the statement you used would always return true

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the PR is WIP / draft as i debug something all those will be dropped
not ready to review (saw the failure just after i asked sorry)
thanks

Copy link
Collaborator Author

@oshoval oshoval Jul 3, 2024

Choose a reason for hiding this comment

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

right, was fixed later thanks,
but anyhow it will be dropped before this PR is merged, just poced to see it fix the root problem
(dropped)

@oshoval oshoval changed the title kubevirtci: Bump version WIP kubevirtci: Bump version Jul 3, 2024
@oshoval oshoval marked this pull request as ready for review July 3, 2024 14:26
@oshoval
Copy link
Collaborator Author

oshoval commented Jul 3, 2024

hold would be removed once the last commit is dropped, atm it is there just to see the fixes it poced
helps

@oshoval oshoval force-pushed the bumpkci branch 3 times, most recently from 84fe039 to 95c949a Compare July 4, 2024 06:24
@oshoval oshoval marked this pull request as draft July 4, 2024 06:24
@oshoval oshoval changed the title WIP kubevirtci: Bump version kubevirtci: Bump version Jul 4, 2024
Also allow to override KUBEVIRTCI_TAG.

Signed-off-by: Or Shoval <[email protected]>
The default value at cluster.sh already has the right value.
One source of truth is much better.

Signed-off-by: Or Shoval <[email protected]>
@oshoval oshoval marked this pull request as ready for review July 4, 2024 11:30
@kubevirt-bot kubevirt-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 4, 2024
@oshoval
Copy link
Collaborator Author

oshoval commented Jul 4, 2024

/hold cancel

@kubevirt-bot kubevirt-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 4, 2024
@oshoval oshoval requested a review from phoracek July 4, 2024 11:32
@oshoval
Copy link
Collaborator Author

oshoval commented Jul 4, 2024

comments were addressed, all debug code was remove, and at the end we don't need to pin OVS
to an old version, as the root cause was fixed
(wrong kubevirtci tag was used when running OVS e2e tests,
the right tag should be according the actual deployed cluster, i.e the one CNAO uses)

This change was done for KMP and OVS, and should be done for the rest of the repos, but it doesnt block this PR
Example k8snetworkplumbingwg/kubemacpool#432

@oshoval
Copy link
Collaborator Author

oshoval commented Jul 4, 2024

all passed, addressed comments,
now the PR has the minimal trivial required changes for a bump

/lgtm
/approve

@kubevirt-bot
Copy link
Collaborator

@oshoval: you cannot LGTM your own PR.

In response to this:

all passed, addressed comments,
now the PR has the minimal trivial required changes for a bump

/lgtm
/approve

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.

@kubevirt-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: oshoval

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 Jul 4, 2024
@oshoval oshoval added the lgtm Indicates that a PR is ready to be merged. label Jul 4, 2024
@kubevirt-bot kubevirt-bot merged commit 8f67002 into kubevirt:main Jul 4, 2024
15 checks passed
nestoracunablanco added a commit to nestoracunablanco/cluster-network-addons-operator that referenced this pull request Sep 4, 2024
nestoracunablanco added a commit to nestoracunablanco/cluster-network-addons-operator that referenced this pull request Sep 10, 2024
nestoracunablanco added a commit to nestoracunablanco/cluster-network-addons-operator that referenced this pull request Sep 17, 2024
nestoracunablanco added a commit to nestoracunablanco/cluster-network-addons-operator that referenced this pull request Sep 23, 2024
nestoracunablanco added a commit to nestoracunablanco/cluster-network-addons-operator that referenced this pull request Sep 25, 2024
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/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants