Skip to content

cluster: Fix kubevirtci clone storm#2619

Merged
kubevirt-bot merged 1 commit intokubevirt:mainfrom
ormergi:cluster-up-fix
Mar 10, 2026
Merged

cluster: Fix kubevirtci clone storm#2619
kubevirt-bot merged 1 commit intokubevirt:mainfrom
ormergi:cluster-up-fix

Conversation

@ormergi
Copy link
Copy Markdown
Contributor

@ormergi ormergi commented Mar 10, 2026

What this PR does / why we need it:
Every time cluster/ scrips are called, it checks if kubevirtci repo exist.
The check expects for kubevirtci repo remote to be

The result is kubevirtci clone is being deleted along with the cluster kubeconfig, and then cloned again.
Making the dev env cluster in-accessible and the dev workflow impossible.

Change clone existence check to look for the clone commit diff only and not the remote URL.

This change should avoid cluster/ scripts to mistakenly remove kubevirtci clones.

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 Mar 10, 2026
@kubevirt-bot kubevirt-bot requested review from oshoval and qinqon March 10, 2026 11:08
Copy link
Copy Markdown
Collaborator

@oshoval oshoval left a comment

Choose a reason for hiding this comment

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

Interesting, wasn't the state, seems a regression

i wonder if it works also for someone that doesn't have ssh key / gh logged in etc ?
worth to check please if possible

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Mar 10, 2026
@oshoval
Copy link
Copy Markdown
Collaborator

oshoval commented Mar 10, 2026

@kubevirt-bot kubevirt-bot removed the lgtm Indicates that a PR is ready to be merged. label Mar 10, 2026
@ormergi
Copy link
Copy Markdown
Contributor Author

ormergi commented Mar 10, 2026

Interesting, wasn't the state, seems a regression

i wonder if it works also for someone that doesn't have ssh key / gh logged in etc ? worth to check please if possible

Devs should be logged in to GH in order to contribute for this project, CI will clone it anyways.
I am not sure what you looking for.

@ormergi
Copy link
Copy Markdown
Contributor Author

ormergi commented Mar 10, 2026

/lgtm cancel

might be related ? https://prow.ci.kubevirt.io/view/gs/kubevirt-prow/pr-logs/pull/kubevirt_cluster-network-addons-operator/2619/pull-e2e-cluster-network-addons-operator-workflow-k8s-s390x/2031326686074638336

also worth to check please as previous comment says

Looks like these errors coming from kubeadm kubevirtci use to create the cluster. I have no clue why though..

@ormergi
Copy link
Copy Markdown
Contributor Author

ormergi commented Mar 10, 2026

@oshoval why do we even bother to remove _kubevirtci dir, cant we just leave it?

EDIT:
It seems that on CI the remote URL is not in SSH form
https://prow.ci.kubevirt.io/view/gs/kubevirt-prow/pr-logs/pull/kubevirt_cluster-network-addons-operator/2619/pull-e2e-cluster-network-addons-operator-workflow-k8s/2031326663463145472#1:build-log.txt%3A98

@ormergi ormergi force-pushed the cluster-up-fix branch 3 times, most recently from 4ea8d21 to 182700d Compare March 10, 2026 11:37
@oshoval
Copy link
Copy Markdown
Collaborator

oshoval commented Mar 10, 2026

Devs should be logged in to GH in order to contribute for this project, CI will clone it anyways.
I am not sure what you looking for.

a user might want to clone without creating PRs, we should allow it

@ormergi
Copy link
Copy Markdown
Contributor Author

ormergi commented Mar 10, 2026

@oshoval I pushed new changes to make the clone condition to not look for the remote URL, PTAL
The project clone kubevirtci and expect it to be there, it should not account for tinkering with the clone dir.

@oshoval
Copy link
Copy Markdown
Collaborator

oshoval commented Mar 10, 2026

nice thanks,
please update PR desc

/lgtm
/approve

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Mar 10, 2026
@kubevirt-bot
Copy link
Copy Markdown
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

Details 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 Mar 10, 2026
@ormergi
Copy link
Copy Markdown
Contributor Author

ormergi commented Mar 10, 2026

Done

@oshoval
Copy link
Copy Markdown
Collaborator

oshoval commented Mar 10, 2026

Thanks

@oshoval
Copy link
Copy Markdown
Collaborator

oshoval commented Mar 10, 2026

btw we might suffer from the same on more network repos, since they have common scripts in this area

oshoval added a commit to oshoval/bridge-marker that referenced this pull request Mar 10, 2026
cluster::install checks whether the existing kubevirtci clone matches
the requested one by comparing both the remote URL and the tag.

The remote URL check fails when the clone was done via SSH
(git@github.com:...) because the stored KUBEVIRTCI_REPO uses HTTPS
(https://github.com/...). The mismatch causes kubevirtci to be deleted
and re-cloned on every invocation, wiping the cluster kubeconfig and
breaking the dev workflow.

Remove the remote URL check and keep only the tag comparison.

Derived from: kubevirt/cluster-network-addons-operator#2619
Assisted-by: Cursor (claude-4.6-sonnet-medium-thinking)
Signed-off-by: Or Shoval <oshoval@redhat.com>
oshoval added a commit to oshoval/ovs-cni that referenced this pull request Mar 10, 2026
cluster::install checks whether the existing kubevirtci clone matches
the requested one by comparing both the remote URL and the tag.

The remote URL check fails when the clone was done via SSH
(git@github.com:...) because the stored KUBEVIRTCI_REPO uses HTTPS
(https://github.com/...). The mismatch causes kubevirtci to be deleted
and re-cloned on every invocation, wiping the cluster kubeconfig and
breaking the dev workflow.

Remove the remote URL check and keep only the tag comparison.

Derived from: kubevirt/cluster-network-addons-operator#2619
Assisted-by: Cursor (claude-4.6-sonnet-medium-thinking)
Signed-off-by: Or Shoval <oshoval@redhat.com>
oshoval added a commit to oshoval/kubemacpool that referenced this pull request Mar 10, 2026
cluster::install checks whether the existing kubevirtci clone matches
the requested one by comparing both the remote URL and the tag.

The remote URL check fails when the clone was done via SSH
(git@github.com:...) because the stored KUBEVIRTCI_REPO uses HTTPS
(https://github.com/...). The mismatch causes kubevirtci to be deleted
and re-cloned on every invocation, wiping the cluster kubeconfig and
breaking the dev workflow.

Remove the remote URL check and keep only the tag comparison.

Derived from: kubevirt/cluster-network-addons-operator#2619
Assisted-by: Cursor (claude-4.6-sonnet-medium-thinking)
Signed-off-by: Or Shoval <oshoval@redhat.com>
@oshoval
Copy link
Copy Markdown
Collaborator

oshoval commented Mar 10, 2026

bridge-marker: kubevirt/bridge-marker#98
kubemacpool: k8snetworkplumbingwg/kubemacpool#601
ovs-cni: k8snetworkplumbingwg/ovs-cni#441

The change should be safe, once it pass on CI we are good to go imho

@oshoval
Copy link
Copy Markdown
Collaborator

oshoval commented Mar 10, 2026

please drop unused var

https://github.com/ormergi/cluster-network-addons-operator/blob/182700db124cd15b090e22a28aff2d9ebd49c09d/cluster/cluster.sh#L18

oshoval added a commit to oshoval/bridge-marker that referenced this pull request Mar 10, 2026
cluster::install checks whether the existing kubevirtci clone matches
the requested one by comparing both the remote URL and the tag.

The remote URL check fails when the clone was done via SSH
(git@github.com:...) because the stored KUBEVIRTCI_REPO uses HTTPS
(https://github.com/...). The mismatch causes kubevirtci to be deleted
and re-cloned on every invocation, wiping the cluster kubeconfig and
breaking the dev workflow.

Remove the remote URL check and keep only the tag comparison.
Drop the now-unused KUBEVIRTCI_REPO variable.

Derived from: kubevirt/cluster-network-addons-operator#2619
Assisted-by: Cursor (claude-4.6-sonnet-medium-thinking)
Signed-off-by: Or Shoval <oshoval@redhat.com>
oshoval added a commit to oshoval/kubemacpool that referenced this pull request Mar 10, 2026
cluster::install checks whether the existing kubevirtci clone matches
the requested one by comparing both the remote URL and the tag.

The remote URL check fails when the clone was done via SSH
(git@github.com:...) because the stored KUBEVIRTCI_REPO uses HTTPS
(https://github.com/...). The mismatch causes kubevirtci to be deleted
and re-cloned on every invocation, wiping the cluster kubeconfig and
breaking the dev workflow.

Remove the remote URL check and keep only the tag comparison.
Drop the now-unused KUBEVIRTCI_REPO variable.

Derived from: kubevirt/cluster-network-addons-operator#2619
Assisted-by: Cursor (claude-4.6-sonnet-medium-thinking)
Signed-off-by: Or Shoval <oshoval@redhat.com>
oshoval added a commit to oshoval/ovs-cni that referenced this pull request Mar 10, 2026
cluster::install checks whether the existing kubevirtci clone matches
the requested one by comparing both the remote URL and the tag.

The remote URL check fails when the clone was done via SSH
(git@github.com:...) because the stored KUBEVIRTCI_REPO uses HTTPS
(https://github.com/...). The mismatch causes kubevirtci to be deleted
and re-cloned on every invocation, wiping the cluster kubeconfig and
breaking the dev workflow.

Remove the remote URL check and keep only the tag comparison.
Drop the now-unused KUBEVIRTCI_REPO variable.

Derived from: kubevirt/cluster-network-addons-operator#2619
Assisted-by: Cursor (claude-4.6-sonnet-medium-thinking)
Signed-off-by: Or Shoval <oshoval@redhat.com>
Every time cluster/ scrips are called, it checks if kubevirtci
repo exist.
The check expects for kubevirtci repo remote to be:
  https://github.com/...
In practice when git clone a repository the default origin
remote URL will be in SSH form:
  git@github.com:...

The result is kubevirtci clone is being deleted along with the
cluster kubeconfig, and then cloned again.
Making the dev env cluster in-accessible and the dev workflow
impossible.

cluster::install checks the clone remote URL and commit.
There is no actual need to check the clone remote URL because
it clone kubevirtci upstream.
The project should not account for tinkering with the clone dir.
Hence:

Change cluster::install check to look for the clone commit diff only.

This change should avoid cluster/ scripts to mistakenly remove
kubevirtci clones.

Signed-off-by: Or Mergi <ormergi@redhat.com>
@kubevirt-bot kubevirt-bot removed the lgtm Indicates that a PR is ready to be merged. label Mar 10, 2026
@ormergi
Copy link
Copy Markdown
Contributor Author

ormergi commented Mar 10, 2026

@sonarqubecloud
Copy link
Copy Markdown

@oshoval
Copy link
Copy Markdown
Collaborator

oshoval commented Mar 10, 2026

Thanks
/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Mar 10, 2026
@kubevirt-bot kubevirt-bot merged commit 5b3e87f into kubevirt:main Mar 10, 2026
16 checks passed
kubevirt-bot pushed a commit to k8snetworkplumbingwg/kubemacpool that referenced this pull request Mar 11, 2026
cluster::install checks whether the existing kubevirtci clone matches
the requested one by comparing both the remote URL and the tag.

The remote URL check fails when the clone was done via SSH
(git@github.com:...) because the stored KUBEVIRTCI_REPO uses HTTPS
(https://github.com/...). The mismatch causes kubevirtci to be deleted
and re-cloned on every invocation, wiping the cluster kubeconfig and
breaking the dev workflow.

Remove the remote URL check and keep only the tag comparison.
Drop the now-unused KUBEVIRTCI_REPO variable.

Derived from: kubevirt/cluster-network-addons-operator#2619
Assisted-by: Cursor (claude-4.6-sonnet-medium-thinking)

Signed-off-by: Or Shoval <oshoval@redhat.com>
@ormergi ormergi deleted the cluster-up-fix branch March 18, 2026 15:14
kubevirt-bot pushed a commit to kubevirt/bridge-marker that referenced this pull request Mar 23, 2026
cluster::install checks whether the existing kubevirtci clone matches
the requested one by comparing both the remote URL and the tag.

The remote URL check fails when the clone was done via SSH
(git@github.com:...) because the stored KUBEVIRTCI_REPO uses HTTPS
(https://github.com/...). The mismatch causes kubevirtci to be deleted
and re-cloned on every invocation, wiping the cluster kubeconfig and
breaking the dev workflow.

Remove the remote URL check and keep only the tag comparison.
Drop the now-unused KUBEVIRTCI_REPO variable.

Derived from: kubevirt/cluster-network-addons-operator#2619
Assisted-by: Cursor (claude-4.6-sonnet-medium-thinking)

Signed-off-by: Or Shoval <oshoval@redhat.com>
kubevirt-bot pushed a commit to k8snetworkplumbingwg/ovs-cni that referenced this pull request Mar 31, 2026
cluster::install checks whether the existing kubevirtci clone matches
the requested one by comparing both the remote URL and the tag.

The remote URL check fails when the clone was done via SSH
(git@github.com:...) because the stored KUBEVIRTCI_REPO uses HTTPS
(https://github.com/...). The mismatch causes kubevirtci to be deleted
and re-cloned on every invocation, wiping the cluster kubeconfig and
breaking the dev workflow.

Remove the remote URL check and keep only the tag comparison.
Drop the now-unused KUBEVIRTCI_REPO variable.

Derived from: kubevirt/cluster-network-addons-operator#2619
Assisted-by: Cursor (claude-4.6-sonnet-medium-thinking)

Signed-off-by: Or Shoval <oshoval@redhat.com>
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/XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants