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.

Sign up for GitHub

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

Jump to bottom

Add multus bump script #456

Merged
merged 3 commits into from
Jul 22, 2020
Merged

Add multus bump script #456

merged 3 commits into from
Jul 22, 2020

Conversation

RamLavi
Copy link
Collaborator

@RamLavi RamLavi commented Jul 8, 2020 •
edited
Loading

What this PR does / why we need it:
Currently cnao only bumps multus manually.
Let's add a script to do it for us.

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 Jul 8, 2020
@RamLavi
Copy link
Collaborator Author

RamLavi commented Jul 8, 2020

/hold

until #449 is merged

@kubevirt-bot kubevirt-bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jul 8, 2020
@kubevirt-bot kubevirt-bot added size/M and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L labels Jul 12, 2020
@ormergi
Copy link
Contributor

ormergi commented Jul 13, 2020

/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Jul 13, 2020
phoracek
phoracek requested changes Jul 13, 2020
View reviewed changes
Copy link
Member

@phoracek phoracek left a comment

Choose a reason for hiding this comment

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

/hold

Can we also fetch Multus manifests?

@kubevirt-bot kubevirt-bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 13, 2020
@RamLavi
Copy link
Collaborator Author

RamLavi commented Jul 13, 2020

/hold

Can we also fetch Multus manifests?

I think we can use the image yaml with some kustomize/sed tweeks

Though I didn't undersatnd why they have 2 multus daemonsets there

@alonSadan
Copy link
Contributor

/lgtm

@kubevirt-bot kubevirt-bot added lgtm Indicates that a PR is ready to be merged. and removed lgtm Indicates that a PR is ready to be merged. size/S labels Jul 13, 2020
@RamLavi RamLavi force-pushed the add_multus_script branch 3 times, most recently from 59cd5d0 to 14bd37d Compare July 20, 2020 16:41
@qinqon
Copy link
Collaborator

qinqon commented Jul 20, 2020

/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Jul 20, 2020
@phoracek
Copy link
Member

/retest
/approve

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 20, 2020
@RamLavi
Copy link
Collaborator Author

RamLavi commented Jul 20, 2020

/hold

need to resolve the '' added to the replaced names

@kubevirt-bot kubevirt-bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 21, 2020
@RamLavi
Copy link
Collaborator Author

RamLavi commented Jul 21, 2020

/unhold

@kubevirt-bot kubevirt-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 21, 2020
@qinqon
Copy link
Collaborator

qinqon commented Jul 21, 2020

/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Jul 21, 2020
@RamLavi RamLavi mentioned this pull request Jul 21, 2020
Closed
@RamLavi
Copy link
Collaborator Author

RamLavi commented Jul 21, 2020

/retest

unrelated flakyness

@phoracek
Copy link
Member

/retest

@phoracek
Copy link
Member

phoracek commented Jul 21, 2020 •
edited
Loading

/hold

There is a real issue:

      message: 'could not apply (apps/v1, Kind=DaemonSet) cluster-network-addons/multus:
        could not update object (apps/v1, Kind=DaemonSet) cluster-network-addons/multus:
        DaemonSet.apps "multus" is invalid: spec.selector: Invalid value: v1.LabelSelector{MatchLabels:map[string]string{"name":"multus"},
        MatchExpressions:[]v1.LabelSelectorRequirement(nil)}: field is immutable'

Seems that the new version of the DaemonSet changes label selectors. We are not allowed to patch that live.

@kubevirt-bot kubevirt-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 21, 2020
@RamLavi
add manifest creation by the multus bump script
449f48e 
Signed-off-by: Ram Lavi <ralavi@redhat.com>
@kubevirt-bot kubevirt-bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 22, 2020
@RamLavi
Copy link
Collaborator Author

RamLavi commented Jul 22, 2020

Seems that the new version of the DaemonSet changes label selectors. We are not allowed to patch that live.

From some reason it doesn't recreate in my local, but I changed it as the error is very specific. Let's see if this solves the issue

@phoracek
Copy link
Member

/hold

@phoracek
Copy link
Member

/hold cancel
/lgtm
/approve

@kubevirt-bot kubevirt-bot added lgtm Indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Jul 22, 2020
@kubevirt-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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 merged commit 42c4471 into kubevirt:master Jul 22, 2020
@phoracek phoracek mentioned this pull request Jul 22, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qinqon qinqon qinqon requested changes

@oshoval oshoval oshoval left review comments

@phoracek phoracek phoracek requested changes

Assignees

@qinqon qinqon

@phoracek phoracek

@ormergi ormergi

@alonSadan alonSadan

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/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@RamLavi @ormergi @alonSadan @qinqon @phoracek @kubevirt-bot @oshoval