Skip to content
This repository was archived by the owner on Feb 6, 2025. It is now read-only.

Forcing to use rev5 for cilium image for bugfix (bsc#1173039) #1197

Merged
merged 1 commit into from
Jul 9, 2020
Merged

Forcing to use rev5 for cilium image for bugfix (bsc#1173039) #1197

merged 1 commit into from
Jul 9, 2020

Conversation

mmnelemane
Copy link
Contributor

Why is this PR needed?

Fixes #1173039

What does this PR do?

Provides caasp-cilium-image revision update for the bug fix for bsc#1173039

Anything else a reviewer needs to know?

Info for QA

  • Ensure skuba downloads the correct image with rev5 label
  • Test the image for resolution of the bug described in bsc#1173039

Related info

Info that can be relevant for QA:

Status BEFORE applying the patch

Status AFTER applying the patch

Docs

Merge restrictions

(Please do not edit this)

We are in v4-maintenance phase, so we will restrict what can be merged to prevent unexpected surprises:

What can be merged (merge criteria):
    2 approvals:
        1 developer: code is fine
        1 QA: QA is fine
    there is a PR for updating documentation (or a statement that this is not needed)

jenting
jenting reviewed Jun 29, 2020
View reviewed changes
Copy link

@jenting jenting left a comment

Choose a reason for hiding this comment

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

Manifest version bump is also needed :)

@mmnelemane
Copy link
Contributor Author

Manifest version bump is also needed :)

@jenting can you please point me to how a manifest version bump should be done ?

jenting
jenting reviewed Jul 2, 2020
View reviewed changes
Copy link

@jenting jenting left a comment

Choose a reason for hiding this comment

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

refs to #1018 (review)

@mmnelemane mmnelemane force-pushed the cilium_bugfix_1173039 branch from 3567be8 to 60b1efa Compare July 3, 2020 10:14
@mmnelemane
Copy link
Contributor Author

Conflict because master already uses 1.7.5 and this bugfix for caasp4 has nowhere to go unless a new branch (release-caasp-4.2.2) is created.

@mmnelemane mmnelemane changed the base branch from master to release-caasp-4.2.2 July 3, 2020 10:43
@mmnelemane mmnelemane force-pushed the cilium_bugfix_1173039 branch 2 times, most recently from 7329299 to 122a237 Compare July 3, 2020 11:43
@mmnelemane mmnelemane changed the base branch from release-caasp-4.2.2 to maintenance-caasp-v4 July 3, 2020 11:44
jenting
jenting previously approved these changes Jul 3, 2020
View reviewed changes
Copy link

@jenting jenting left a comment

Choose a reason for hiding this comment

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

LGTM although the unit test image tag doesn’t need to bump the version.

jordimassaguerpla
jordimassaguerpla suggested changes Jul 3, 2020
View reviewed changes
Copy link
Member

@jordimassaguerpla jordimassaguerpla left a comment

Choose a reason for hiding this comment

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

From what I see here, we have cilium 1.5.3 in CaaSPv4. Upgrading to 1.6.6 for adding a patch seems a bit overkill. Can't the patch be applied to 1.5.3 or what am I missing ?

@jordimassaguerpla
Copy link
Member

From what I see here, we have cilium 1.5.3 in CaaSPv4. Upgrading to 1.6.6 for adding a patch seems a bit overkill. Can't the patch be applied to 1.5.3 or what am I missing ?

I will answer myself. The 1.5.3 are in the "test" files. The versions.go contain the 1.6.6

LGTM

@jordimassaguerpla
Copy link
Member

I just checked the failing tests. They are about these tests you are changing. Could you please review the tests?

@mmnelemane mmnelemane dismissed stale reviews from jordimassaguerpla and jenting via 9f67c90 July 3, 2020 15:09
@mmnelemane mmnelemane force-pushed the cilium_bugfix_1173039 branch 2 times, most recently from 9f67c90 to 5357bcd Compare July 3, 2020 15:13
@mmnelemane
Copy link
Contributor Author

Now its much simplified, since the versions in the test files are taken care of by @mrostecki in #1223 and #1224

jenting
jenting previously approved these changes Jul 4, 2020
View reviewed changes
soumynathan
soumynathan reviewed Jul 6, 2020
View reviewed changes
Copy link

@soumynathan soumynathan left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@mmnelemane mmnelemane dismissed stale reviews from jenting and jordimassaguerpla via 442aa20 July 6, 2020 17:04
@mmnelemane mmnelemane force-pushed the cilium_bugfix_1173039 branch from 5357bcd to 442aa20 Compare July 6, 2020 17:04
@mmnelemane mmnelemane removed the request for review from vadorovsky July 6, 2020 17:05
@mmnelemane mmnelemane force-pushed the cilium_bugfix_1173039 branch from 442aa20 to eeb1941 Compare July 7, 2020 15:37
@mmnelemane
Copy link
Contributor Author

SUSE/caasp-container-images#101 could help fix the skuba-test issue of missing cilium-operator image.

@mmnelemane mmnelemane merged commit 6288ace into SUSE:maintenance-caasp-v4 Jul 9, 2020
mkoci-suse pushed a commit that referenced this pull request Feb 6, 2025
@mmnelemane
Merge pull request #1197 from mmnelemane/cilium_bugfix_1173039
6b97300 
Forcing to use rev5 for cilium image for bugfix (bsc#1173039)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@soumynathan soumynathan soumynathan left review comments

@vadorovsky vadorovsky vadorovsky approved these changes

@jenting jenting jenting approved these changes

@jordimassaguerpla jordimassaguerpla jordimassaguerpla left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@mmnelemane @jordimassaguerpla @soumynathan @vadorovsky @jenting