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

feat: add rke2 network configuration to installer #886

Merged
merged 3 commits into from
Dec 18, 2024

Conversation

ihcsim
Copy link
Contributor

@ihcsim ihcsim commented Nov 29, 2024

Problem:

The RKE2 pod, service and DNS IP settings are hard-coded in the rke2-90-harvester-server.yaml file, making it difficult for users to resolve any CIDR conflicts in their environment.

Solution:

Update the installer to include a screen to configure the RKE2 pod, service and DNS IP settings. This customization is only possible during the initial installation. Modification of these settings post-installation isn't supported.

See RKE2 doc for the description of these parameters: cluster-cidr, service-cidr and cluster-dns.

Related Issue:

Fix harvester/harvester#4254.

Test plan:

Test Case 1 - Override cluster network configuration

  • Run the installer to provision a 3-node Harvester cluster with 1 management node and 2 worker nodes
  • Change the cluster network configuration on the 'Configure cluster network' screen:

image

  • Expect the cluster nodes to come up in a healthy state and the node IP CIDR is unaffected by the customization:
$ kubectl get node -owide
NAME       STATUS   ROLES                       AGE     VERSION          INTERNAL-IP       EXTERNAL-IP   OS-IMAGE           KERNEL-VERSION                 CONTAINER-RUNTIME
dev-core   Ready    control-plane,etcd,master   20m     v1.29.9+rke2r1   192.168.122.131   <none>        Harvester master   5.14.21-150500.55.83-default   containerd://1.7.21-k3s2
dev-wkr1   Ready    <none>                      8m58s   v1.29.9+rke2r1   192.168.122.14    <none>        Harvester master   5.14.21-150500.55.83-default   containerd://1.7.21-k3s2
dev-wkr2   Ready    <none>                      25s     v1.29.9+rke2r1   192.168.122.23    <none>        Harvester master   5.14.21-150500.55.83-default   containerd://1.7.21-k3s2
  • Ensure all pods, except those using host network, are within the 172.16.0.0/16 range: kubectl get po -A -owide
  • Ensure all services are within the 172.22.0.0/16 range: kubectl get svc -A -owide
  • Ensure the Core DNS service is assigned the 172.22.0.10 IP address:
kubectl -n kube-system get svc rke2-coredns-rke2-coredns
NAME                        TYPE        CLUSTER-IP    EXTERNAL-IP   PORT(S)         AGE
rke2-coredns-rke2-coredns   ClusterIP   172.22.0.10   <none>        53/UDP,53/TCP   24m
  • Deploy a Nginx deployment, service and PVC. YAML at gist here.
  • Expect the pod and service be assigned the in-range IP addresses:
NAME                        READY   STATUS    RESTARTS   AGE     IP            NODE       NOMINATED NODE   READINESS GATES
pod/nginx-c85947998-twfpj   1/1     Running   0          3m12s   172.16.2.18   dev-wkr2   <none>           <none>

NAME                 TYPE        CLUSTER-IP      EXTERNAL-IP   PORT(S)   AGE    SELECTOR
service/kubernetes   ClusterIP   172.22.0.1      <none>        443/TCP   29m    <none>
service/nginx        ClusterIP   172.22.97.156   <none>        80/TCP    108s   app=nginx

NAME                               STATUS   VOLUME                                     CAPACITY   ACCESS MODES   STORAGECLASS         VOLUMEATTRIBUTESCLASS   AGE     VOLUMEMODE
persistentvolumeclaim/nginx-data   Bound    pvc-6f6321f4-fc94-4743-8e8c-99957798620e   5Gi        RWO            harvester-longhorn   <unset>                 3m12s   Filesystem

NAME                                                        CAPACITY   ACCESS MODES   RECLAIM POLICY   STATUS   CLAIM                STORAGECLASS         VOLUMEATTRIBUTESCLASS   REASON   AGE     VOLUMEMODE
persistentvolume/pvc-6f6321f4-fc94-4743-8e8c-99957798620e   5Gi        RWO            Delete           Bound    default/nginx-data   harvester-longhorn   <unset>                          3m10s   Filesystem

Test Case 2 - Use default cluster network configuration

  • Run the installer to provision a 3-node Harvester cluster with 1 management node and 2 worker nodes
  • Leave the cluster network configuration on the 'Configure cluster network' screen blank
  • Expect Harvester cluster to come up in a healthy state
  • Expect all pods and services to use the default CIDR settings defined in the rke2-90-harvester-server.yaml file

Additional Validation

  • The 'Configure cluster network' screen only shows up in the 'Create a new Harvester cluster' installation flow
    • This new screen should not show up in the 'Join a existing cluster' flow (as user traverses back-and-forth between screens)
  • The pod CIDR and service CIDR must be well-formed. Otherwise, error messages will be shown on the 'Configure cluster network' screen.
  • If the service CIDR is changed, the DNS IP must also be updated to be within the service CIDR range. Otherwise, error messages will be shown on the 'Configure cluster network' screen

@ihcsim ihcsim self-assigned this Nov 29, 2024
@ihcsim ihcsim requested a review from bk201 November 29, 2024 20:26
Copy link
Contributor

@ibrokethecloud ibrokethecloud 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 for the PR.
I was able to test by creating clusters with and without custom pod/service cidr ranges.

These overridden CIDR settings will be injected into the promote.sh
data in the harvester-helper config map.

Signed-off-by: Ivan Sim <[email protected]>
ihcsim added a commit to ihcsim/harvester that referenced this pull request Dec 13, 2024
…otion

config

During installation, these custom values would have been added to the
Harvester's ManagedChart config at
/etc/rancher/rancherd/config.yaml.d/10-harvester.yaml. See
harvester/harvester-installer#886.

Signed-off-by: Ivan Sim <[email protected]>
ihcsim added a commit to ihcsim/harvester that referenced this pull request Dec 13, 2024
During installation, these custom values would have come from
the Harvester's ManagedChart config at
/etc/rancher/rancherd/config.yaml.d/10-harvester.yaml. See
harvester/harvester-installer#886.

Signed-off-by: Ivan Sim <[email protected]>
FrankYang0529 pushed a commit to harvester/harvester that referenced this pull request Dec 18, 2024
During installation, these custom values would have come from
the Harvester's ManagedChart config at
/etc/rancher/rancherd/config.yaml.d/10-harvester.yaml. See
harvester/harvester-installer#886.

Signed-off-by: Ivan Sim <[email protected]>
Copy link
Member

@w13915984028 w13915984028 left a comment

Choose a reason for hiding this comment

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

two questions, thanks.

@@ -202,6 +202,10 @@ resources:
enabled: true
kube-vip-cloud-provider:
enabled: true
promote:
Copy link
Member

Choose a reason for hiding this comment

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

Is RKE2 better than promote?

we also have another requirement: customize the RKE2 CNI, some one prefers Calico

Copy link
Contributor Author

@ihcsim ihcsim Dec 18, 2024

Choose a reason for hiding this comment

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

My initial thought is to communicate that, within the ManagedChart, these settings are only used by the promotion controller, instead of a general mechanism to override RKE2, per harvester/harvester#7156. My current preference leans towards keeping it as promote. LMKWYT.

Re: CNI, can we work on it as a separate issue?

Copy link
Member

Choose a reason for hiding this comment

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

Sure, customized CNI will be in another issue & PR, which needs some investigation.

promotion is also good.

Copy link
Contributor Author

@ihcsim ihcsim Dec 18, 2024

Choose a reason for hiding this comment

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

Custom CNI enhancement request added at harvester/harvester#7197.

@@ -202,6 +202,10 @@ resources:
enabled: true
kube-vip-cloud-provider:
enabled: true
promote:
clusterPodCIDR: {{ or .ClusterPodCIDR "10.52.0.0/16" }}
clusterServiceCIDR: {{ or .ClusterServiceCIDR "10.53.0.0/16" }}
Copy link
Member

Choose a reason for hiding this comment

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

When users customize/default those 3 fields, the value will be writen to harvester managedchart, could you check if we can add webhook to deny any later change on the managedchart related fields? thanks.

If a user tries to change the managedchart on the fly, the result will be unexpected.

btw, do you have a document PR to for those installation configuration options.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point on validation webhook. I don't see why not. I'll create a new issue for it. The document PR is still WIP.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created harvester/harvester#7196 to track validating webhook implementation.

Copy link
Member

@w13915984028 w13915984028 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.

@ihcsim ihcsim merged commit f3ce10e into harvester:master Dec 18, 2024
7 checks passed
@ihcsim ihcsim deleted the rke2-cidr branch December 18, 2024 21:57
tayterz2 pushed a commit to tayterz2/harvester-installer that referenced this pull request Dec 30, 2024
* Add rke2 network configuration to installer
* Declutter input panel labels
* Add CIDR settings to promote controller chart config

These overridden CIDR settings will be injected into the promote.sh
data in the harvester-helper config map.

Signed-off-by: Ivan Sim <[email protected]>

---------

Signed-off-by: Ivan Sim <[email protected]>
tayterz2 pushed a commit to tayterz2/harvester-installer that referenced this pull request Dec 30, 2024
* Add rke2 network configuration to installer
* Declutter input panel labels
* Add CIDR settings to promote controller chart config

These overridden CIDR settings will be injected into the promote.sh
data in the harvester-helper config map.

Signed-off-by: Ivan Sim <[email protected]>

---------

Signed-off-by: Ivan Sim <[email protected]>
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.

[FEATURE] Modify the default cluster-cidr of harvester
4 participants