Skip to content

Expose service node port range as MicroShift config#649

Merged
oglok merged 1 commit intomainfrom
nodeportrange
Apr 8, 2022
Merged

Expose service node port range as MicroShift config#649
oglok merged 1 commit intomainfrom
nodeportrange

Conversation

@oglok
Copy link
Contributor

@oglok oglok commented Apr 7, 2022

Signed-off-by: Ricardo Noriega rnoriega@redhat.com

With this new parameter, service node port range can be successfully customized as a MicroShift config para:

config.yaml

cluster: 
  url:                  "https://127.0.0.1:6443"
  clusterCIDR:          "10.42.0.0/16"
  serviceCIDR:          "10.44.0.0/16"
  serviceNodePortRange: "1050-33000"
  dns:                  "10.44.0.10"
  domain:               "cluster.local"

We edit the router nodePort from 30001 to 2001:

[root@fedora ~] # oc get svc -A 
NAMESPACE           NAME                        TYPE        CLUSTER-IP      EXTERNAL-IP   PORT(S)                      AGE
default             kubernetes                  ClusterIP   10.44.0.1       <none>        443/TCP                      109s
default             openshift-apiserver         ClusterIP   None            <none>        443/TCP                      68s
default             openshift-oauth-apiserver   ClusterIP   None            <none>        443/TCP                      68s
openshift-dns       dns-default                 ClusterIP   10.44.0.10      <none>        53/UDP,53/TCP,9154/TCP       57s
openshift-ingress   router-external-default     NodePort    10.44.169.17    <none>        80:30001/TCP,443:30002/TCP   57s
openshift-ingress   router-internal-default     ClusterIP   10.44.189.237   <none>        80/TCP,443/TCP,1936/TCP      57s
[root@fedora ~] # oc edit svc router-external-default -n openshift-ingress
service/router-external-default edited
[root@fedora ~] # 
[root@fedora ~] # 
[root@fedora ~] # oc get svc -A 
NAMESPACE           NAME                        TYPE        CLUSTER-IP      EXTERNAL-IP   PORT(S)                     AGE
default             kubernetes                  ClusterIP   10.44.0.1       <none>        443/TCP                     2m25s
default             openshift-apiserver         ClusterIP   None            <none>        443/TCP                     104s
default             openshift-oauth-apiserver   ClusterIP   None            <none>        443/TCP                     104s
openshift-dns       dns-default                 ClusterIP   10.44.0.10      <none>        53/UDP,53/TCP,9154/TCP      93s
openshift-ingress   router-external-default     NodePort    10.44.169.17    <none>        80:2001/TCP,443:30002/TCP   93s
openshift-ingress   router-internal-default     ClusterIP   10.44.189.237   <none>        80/TCP,443/TCP,1936/TCP     93s

Closes #648

Signed-off-by: Ricardo Noriega <rnoriega@redhat.com>
@openshift-ci openshift-ci bot requested review from rootfs and sallyom April 7, 2022 10:51
@oglok oglok requested review from fzdarsky and mangelajo April 7, 2022 10:51
Copy link
Contributor

@mangelajo mangelajo left a comment

Choose a reason for hiding this comment

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

/lgtm

"--cluster-url=" + tt.config.Cluster.URL,
"--cluster-cidr=" + tt.config.Cluster.ClusterCIDR,
"--service-cidr=" + tt.config.Cluster.ServiceCIDR,
"--service-node-port-range=" + tt.config.Cluster.ServiceNodePortRange,
Copy link
Contributor

Choose a reason for hiding this comment

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

Shall we verify the value with a regex to avoid bleeding errors through (harder to track)?

That would probably apply to other parameters btw, so not high prio.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Re-reading the tests, I don't think we expose all these flags via cobra command. So we should do a clean up in a later PR.

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 7, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 7, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mangelajo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details 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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 7, 2022
@oglok oglok merged commit 78be449 into main Apr 8, 2022
@oglok oglok deleted the nodeportrange branch April 8, 2022 11:38
@mangelajo mangelajo mentioned this pull request Jun 10, 2022
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. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Enhancement]: Enable Service Node Port Range as MicroShift config param

2 participants