k8snetworkplumbingwg / ovs-cni

Open vSwitch CNI plugin
Apache License 2.0
224 stars 71 forks source link

multus: Pin image #282

Closed oshoval closed 1 year ago

oshoval commented 1 year ago

Since multus doesnt pin image correctly, pin it before deploying multus.

See https://github.com/k8snetworkplumbingwg/multus-cni/issues/1170

What this PR does / why we need it:

Special notes for your reviewer:

Release note:

None
phoracek commented 1 year ago

/lgtm /approve

kubevirt-bot commented 1 year ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: oshoval, phoracek

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/k8snetworkplumbingwg/ovs-cni/blob/main/OWNERS)~~ [phoracek] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
dougbtv commented 1 year ago

We discussed this during the maintainer's call, and... it's more complex than you might think.

Is there a reason why you can't track master for the OVS-CNI CI? Because I think that might actually be beneficial for both projects.

oshoval commented 1 year ago

Hi For this repo we are using thin, not thick, we don't need thick atm for this repo. This repo doesn't run stuff a lot, so once we do, we prefer to have it stable, with whatever possible pinned and static, so we don't have surprises and can bump only when needed.

Using (as this PR does for thin plugin)

MULTUS_VERSION=v4.0.1
MULTUS_MANIFEST=https://raw.githubusercontent.com/k8snetworkplumbingwg/multus-cni/${MULTUS_VERSION}/deployments/multus-daemonset.yml

with $MULTUS_VERSION as the tag is legit right ? it takes yaml of v4.0.1, and the corresponding image

Note that we never used main on the yaml link, we always pinned the version, which is another reason why to not use latest image with static yaml