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

multus: Pin image #282

Merged
merged 1 commit into from
Oct 3, 2023
Merged

Conversation

oshoval
Copy link
Member

@oshoval oshoval commented Oct 1, 2023

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

See k8snetworkplumbingwg/multus-cni#1170

What this PR does / why we need it:

Special notes for your reviewer:

Release note:

None

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

See k8snetworkplumbingwg/multus-cni#1170

Signed-off-by: Or Shoval <oshoval@redhat.com>
@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 Oct 1, 2023
@phoracek
Copy link
Member

phoracek commented Oct 3, 2023

/lgtm
/approve

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 3, 2023
@kubevirt-bot
Copy link
Collaborator

[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:

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 Oct 3, 2023
@kubevirt-bot kubevirt-bot merged commit 0ed76c9 into k8snetworkplumbingwg:main Oct 3, 2023
3 checks passed
@dougbtv
Copy link
Member

dougbtv commented Oct 12, 2023

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

  • Currently we don't actually have a stable image for thick plugin yet (so that's maybe a to-do item)
  • We currently like having :snapshot images in the main example deployment yaml, because this encourages users to deploy the latest version so that we get feedback more rapidly.
  • My idea about having the git tag match the image tag is not the most straightforward release process, and its bang-to-buck ratio is low (not a lot of benefit for the effort)

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
Copy link
Member Author

oshoval commented Oct 12, 2023

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

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants