Skip to content

Fix api port #459

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

Merged
merged 1 commit into from
Sep 5, 2019
Merged

Fix api port #459

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion controllers/openstackcluster_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,10 +170,17 @@ func (r *OpenStackClusterReconciler) reconcileCluster(logger logr.Logger, cluste
return reconcile.Result{}, errors.Errorf("failed to get control plane machine: %v", err)
}
if controlPlaneMachine != nil {
var apiPort int
if cluster.Spec.ClusterNetwork.APIServerPort == nil {
logger.Info("No API endpoint given, default to 6443")
apiPort = 6443
} else {
apiPort = int(*cluster.Spec.ClusterNetwork.APIServerPort)
}
openStackCluster.Status.APIEndpoints = []infrav1.APIEndpoint{
{
Host: controlPlaneMachine.Spec.FloatingIP,
Port: int(*cluster.Spec.ClusterNetwork.APIServerPort),
Port: apiPort,
},
}
} else {
Expand Down
2 changes: 1 addition & 1 deletion examples/cluster/cluster.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ spec:
# * Disable the apiServerPort property
# single-node control-plane:
# * Enable the apiServerPort property
#apiServerPort: 6443
apiServerPort: <disable when multi-node control-plane>
Copy link
Member

@sbueringer sbueringer Sep 4, 2019

Choose a reason for hiding this comment

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

The rest of the example is commented to support multi-node control plane per default. That's the only property the other way around after this PR. So i would suggest keeping the # at the start of the line

Copy link
Contributor Author

@jichenjc jichenjc Sep 5, 2019

Choose a reason for hiding this comment

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

if you take a look at generated result...
the # will be removed by my kustomize and that's the reason why I have the nil setting and
it lead to panic... (see my changes in the go file)

see my output. all the things marked as comments are removed, guess it's not my kustomize version issue? I am using 3.10 already

apiVersion: cluster.x-k8s.io/v1alpha2
kind: Cluster
metadata:
  name: test-cluster
  namespace: test-cluster
spec:
  clusterNetwork:
    apiServerPort: <disable when multi-node control-plane>
    pods:
      cidrBlocks:
      - 192.168.0.0/16
    serviceDomain: cluster.local
    services:
      cidrBlocks:
      - 10.96.0.0/12
  infrastructureRef:
    apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2
    kind: OpenStackCluster
    name: test-cluster
    namespace: test-cluster
---
apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2
kind: OpenStackCluster
metadata:
  name: test-cluster
  namespace: test-cluster
spec:
  apiServerLoadBalancerAdditionalPorts:
  - 22
  apiServerLoadBalancerFloatingIP: <loadbalancer floating ip>
  apiServerLoadBalancerPort: 6443
  cloudName: openstack
  cloudsSecret:
    name: cloud-config
    namespace: test-cluster
  disablePortSecurity: true
  disableServerTags: true
  dnsNameservers: []
  externalNetworkId: <external-network-id>

Copy link
Member

@sbueringer sbueringer Sep 5, 2019

Choose a reason for hiding this comment

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

Yup the comments are removed through kustomize that's right. So they are not that useful right now. We can add the property for now. I think I would open another PR sooner or later to create separate multi-node & single-node examples with aproprate properties

infrastructureRef:
apiVersion: infrastructure.cluster.x-k8s.io/v1alpha2
kind: OpenStackCluster
Expand Down