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

bump(chart): bump Whereabouts to v0.8.0 #7405

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

WebberHuang1118
Copy link
Member

@WebberHuang1118 WebberHuang1118 commented Jan 20, 2025

Problem:
Bump Whereabout to v0.8.0

Solution:
Bump Whereabout to v0.8.0

Additional Context:
This PR can't be merged until the path of the extra manifest is changed to v1.4.1 (this should happen after v1.4.1 release)

Related Issue:
https://github.com/harvester/security/issues/62

Test plan:
Deploy to running Harvester

  • Building Harvester cluster with v1.4.1-rc1
  • Enabling Storage Network
  • Creating a VM
  • Applying new Whereabouts CRD
    • $ kubectl apply -f deploy/charts/harvester/dependency_charts/whereabouts/crds/whereabouts.cni.cncf.io_ippools.yaml
    • $ kubectl apply -f deploy/charts/harvester/dependency_charts/whereabouts/crds/whereabouts.cni.cncf.io_overlappingrangeipreservations.yaml
  • Updating Whereabouts from v0.7.0 to v0.8.0
    • Adding mangedchart harvester's spec.values with
    whereabouts:
      enabled: true
      image:
        pullPolicy: IfNotPresent
        repository: ghcr.io/k8snetworkplumbingwg/whereabouts
        tag: "v0.8.0"
    
  • Checking if daemoset harvester-whereabouts successfully deployed image ghcr.io/k8snetworkplumbingwg/whereabouts:v0.8.0
  • Checking if Longhorn backing-image-manager-xxxx pods, instance-manager-xxxx pods, and backing-image-ds-xxxx pods (only appearing during uploading) have interface lhnet1 and its IP in the assigned range
  • The VM can stop/restart successfully

Building New Harvester with this PR

  • Building ISO from this branch, and creating the Harvester cluster with the ISO
  • After Harvester brought up, Check if daemonset harvester-whereabouts successfully deploying image ghcr.io/k8snetworkplumbingwg/whereabouts:v0.8.0
  • Enabling Storage Network
  • Checking if Longhorn backing-image-manager-xxxx pods, instance-manager-xxxx pods, and backing-image-ds-xxxx pods (only appearing during uploading) have interface lhnet1 and its IP in the assigned range
  • Checking image upload
  • Creating VM from uploaded image

Upgrade from v1.4.1 to this PR

  • Building Harvester cluster with v1.4.1
  • Enabling Storage Network
  • Creating a VM
  • Upgrading Harvester from v1.4.1 to ISO built from this PR
  • Checking if daemoset harvester-whereabouts successfully deployed image ghcr.io/k8snetworkplumbingwg/whereabouts:v0.8.0
  • Checking if Longhorn backing-image-manager-xxxx pods, instance-manager-xxxx pods, and backing-image-ds-xxxx pods (only appearing during uploading) have interface lhnet1 and its IP in the assigned range
  • The VM can stop/restart successfully

@WebberHuang1118 WebberHuang1118 marked this pull request as draft January 20, 2025 10:34
@WebberHuang1118
Copy link
Member Author

Whereabouts' CRD ippools and overlappingrangeipreservations are not updated in the upgrade path, and this would break LH pods' creation

Found a temporary workaround to move deploy/charts/harvester/dependency_charts/whereabouts/crds into deploy/charts/harvester/dependency_charts/whereabouts/templates, however, this seems to violate the convention

Thanks @starbops for pointing out a similar issue, I will try the proposed method

@WebberHuang1118 WebberHuang1118 changed the title bump(chart): bump Whereabouts to v0.8.0 (DNM) bump(chart): bump Whereabouts to v0.8.0 Jan 22, 2025
@WebberHuang1118 WebberHuang1118 marked this pull request as ready for review January 22, 2025 08:57
@rrajendran17
Copy link
Contributor

@WebberHuang1118 hope we need another PR for this to bump it to v0.8.0

github.com/k8snetworkplumbingwg/whereabouts v0.7.0
.

I will take care of the PR and merge once this PR is merged.

Copy link
Member

@starbops starbops left a comment

Choose a reason for hiding this comment

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

I have a general question: I saw the README in our whereabouts chart mentioned that:

Original Whereabouts CNI Helm charts is out of date. It still used the legacy method to run Whereabouts CNI. We update the charts from Cronjob to DaemonSet to fit new version of Whereabouts CNI.

But when I looked at the upstream repository, it seemed that they do actively maintained the chart in here. There is even a new CRD that we don't include. Do we want to reevaluate whether to use the upstream chart?

@WebberHuang1118
Copy link
Member Author

WebberHuang1118 commented Jan 23, 2025

@WebberHuang1118 hope we need another PR for this to bump it to v0.8.0

github.com/k8snetworkplumbingwg/whereabouts v0.7.0

.
I will take care of the PR and merge once this PR is merged.

Hi @rrajendran17

Do you mean we should have another PR for it?

@WebberHuang1118 Yes I thought so, the current PR is for bumping whereabouts deployment and

github.com/k8snetworkplumbingwg/whereabouts v0.7.0
is for whereabouts code/api usage and it should also be bumped as well. Am I missing something ?

@WebberHuang1118
Copy link
Member Author

WebberHuang1118 commented Jan 23, 2025

I have a general question: I saw the README in our whereabouts chart mentioned that:

Original Whereabouts CNI Helm charts is out of date. It still used the legacy method to run Whereabouts CNI. We update the charts from Cronjob to DaemonSet to fit new version of Whereabouts CNI.

But when I looked at the upstream repository, it seemed that they do actively maintained the chart in here. There is even a new CRD that we don't include. Do we want to reevaluate whether to use the upstream chart?

Hi @starbops

The chart deployment/whereabouts-chart and the new CRD only exist in the master branch, maybe we can refer to this for the next release.

For the manifest update, I currently checked whereabouts/doc.

@WebberHuang1118
Copy link
Member Author

@WebberHuang1118 hope we need another PR for this to bump it to v0.8.0

github.com/k8snetworkplumbingwg/whereabouts v0.7.0

.
I will take care of the PR and merge once this PR is merged.

@rrajendran17
Let's create the related PR once this one is merged, thanks.

@WebberHuang1118 WebberHuang1118 changed the title (DNM) bump(chart): bump Whereabouts to v0.8.0 bump(chart): bump Whereabouts to v0.8.0 Feb 4, 2025
@WebberHuang1118
Copy link
Member Author

WebberHuang1118 commented Feb 4, 2025

Hi @bk201 @starbops @rrajendran17

The extra manifest has been changed to v1.4.1, and I have also re-tested the upgrade path.

If everything is fine, this PR should be ready to be merged. Please help to check it. Thanks.

Copy link
Member

@starbops starbops left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

@rrajendran17
Copy link
Contributor

LGTM,Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants