baremetal: Always use image cache #5008
Conversation
The metal3-image-cache daemonset is started unconditionally by the cluster-baremetal-operator, serving on port 6181. However, we were only pointing at this service in the MachineConfig if the provisioning network was not present. Given that the MachineConfig is fixed at install time while the provisioning network can be added or removed by changing the Provisioning resource, we should always use the cache. This also means the cache will get exercised in CI, where previously it was not.
|
The changes make sense to me, but if the MachineConfig is fixed, then do we end up trying to use the provisioning IP after the network is disabled? Maybe we always need to use the API VIP too, however that's less than ideal since if the user has a provisioning network we want all that traffic to be isolated there. |
|
/retest |
Excellent point. Yes :( I do think this still makes sense if only to make sure we're always exercising the cache in CI.
In general the API VIP shouldn't even be routable from the provisioning network if it exists. |
|
/retest |
Should this be handled in a separate PR? Can we change the MachineConfig when ProvisioningNetwork is disabled on day-2? |
|
/lgtm |
|
/retest |
1 similar comment
|
/retest |
@zaneb Do you know the answer to this? ^^ |
|
That would need to be handled in CBO. We should probably look to address it as part of https://issues.redhat.com/browse/MPINSTALL-1 maybe. |
I wonder if we should support going from enabled->disabled? I've definitely heard requirements to deploy without any provisioning network, then enable it as a day-2 choice, but I've not heard any requirements to do the reverse flow (if you PXE booted all your hosts on day-1, you'll always need the provisioning network to support node rebuild/replacement right?) That said I agree always using the cache makes sense, and we can deal with the potential network-disabled case in future if there turns out to be a requirement for that. /approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hardys 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 |
openshift-ci
bot
added
the
approved
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
I guess if you had a provisioning network but only used virtualmedia on it, then you might decide that was a mistake and want to disable to provisioning network later. You make a good point that supporting this should probably be nobody's top priority though 😆 |
|
@zaneb: The following test failed, say
Full PR test history. Your PR dashboard. 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/test-infra repository. I understand the commands that are listed here. |
The metal3-image-cache daemonset is started unconditionally by the
cluster-baremetal-operator, serving on port 6181. However, we were only
pointing at this service in the MachineConfig if the provisioning
network was not present.
Given that the MachineConfig is fixed at install time while the
provisioning network can be added or removed by changing the
Provisioning resource, we should always use the cache.
This also means the cache will get exercised in CI, where previously it
was not.