search

crc-org / snc

Single Node Cluster creation scripts for OpenShift 4.x as used by CodeReady Containers
https://crc.dev
Apache License 2.0
100 stars 49 forks source link

Stop accessing libvirt over unauthenticated qemu+tcp connection #856

Closed praveenkumar closed 4 months ago

praveenkumar commented 4 months ago

As part of libvirt IPI installation, it was required to have tcp socket enabled for cluster-api to work. We shifted to use SNO and now we don't require it anymore.

praveenkumar commented 4 months ago

/cherry-pick release-4.15

openshift-cherrypick-robot commented 4 months ago

@praveenkumar: once the present PR merges, I will cherry-pick it on top of release-4.15 in a new PR and assign it to you.

In response to [this](https://github.com/crc-org/snc/pull/856#issuecomment-1975928588): >/cherry-pick release-4.15 Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
cfergeau commented 4 months ago

This is unrelated to libvirt socket activation (as in 'systemd socket activation'), this commit stops accessing libvirt over unauthenticated qemu+tcp (which was needed by cluster-api-provider-libvirt), and uses the local/default libvirt connection instead.

praveenkumar commented 4 months ago

accessing libvirt over unauthenticated qemu+tcp

Thanks, updated with the info.

praveenkumar commented 4 months ago 
        # check if firewalld is configured to allow traffic from 192.168.126.0/24 to 192.168.122.1

can be removed (it's already commented out anyway)

        # check that api.${SNC_PRODUCT_NAME}.${BASE_DOMAIN} either can't be resolved, or resolves to 192.168.126.1[01]

This also can be reworked to only check for 192.168.126.11

Should we completely remove this because this was commented out for a long period and we didn't hit any issue around firewalld stuff?

cfergeau commented 4 months ago 
        # check if firewalld is configured to allow traffic from 192.168.126.0/24 to 192.168.122.1

can be removed (it's already commented out anyway)

        # check that api.${SNC_PRODUCT_NAME}.${BASE_DOMAIN} either can't be resolved, or resolves to 192.168.126.1[01]

This also can be reworked to only check for 192.168.126.11

Should we completely remove this because this was commented out for a long period and we didn't hit any issue around firewalld stuff?

We no longer need the VMs to be able to connect to the host libvirtd, so this can be removed. The fact that it was removed is one more reason for the removal :) The DNS resolution check should be kept.

openshift-ci[bot] commented 4 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cfergeau

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: - ~~[OWNERS](https://github.com/crc-org/snc/blob/master/OWNERS)~~ [cfergeau] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
openshift-cherrypick-robot commented 4 months ago

@praveenkumar: new pull request created: #859

In response to [this](https://github.com/crc-org/snc/pull/856#issuecomment-1975928588): >/cherry-pick release-4.15 Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.