Update README to improve clarity and add troubleshooting and FAQs

README.md

@@ -5,9 +5,10 @@
 [![](https://img.shields.io/badge/language-Go-%235adaff.svg?logo=go&style=flat-square "Go language")](https://golang.org)
 [![](https://img.shields.io/docker/v/apache/cloudstack-kubernetes-provider?label=docker%20hub&logo=docker&style=flat-square "Docker Hub Image Version")](https://hub.docker.com/r/apache/cloudstack-kubernetes-provider/)
 
-A Cloud Controller Manager to facilitate Kubernetes deployments on Cloudstack.
+A Cloud Controller Manager to facilitate Kubernetes deployments on CloudStack.
 
-Based on the old Cloudstack provider in Kubernetes was removed.
+It replaces the CloudStack cloud provider that used to be built into Kubernetes and has since
+been removed from the Kubernetes tree.
 
 Refer:
 * https://github.com/kubernetes/kubernetes/tree/release-1.15/pkg/cloudprovider/providers/cloudstack
@@ -39,9 +40,16 @@ api-key = <CloudStack API Key>
 secret-key = <CloudStack API Secret>
 project-id = <CloudStack Project UUID (optional)>
 zone = <CloudStack Zone Name (optional)>
+region = <Region Name (optional)>
 ssl-no-verify = <Disable SSL certificate validation: true or false (optional)>
 ```
 
+If `zone` is not set, it is auto-detected from the node the controller runs on.
+
+`region` sets the value of the region node labels. If it is not set, the region labels use the zone
+name. Some workloads (such as Rook/Ceph) require the zone and region labels to differ. You need to
+explicitly set `region` in that case.
+
 The access token needs to be able to fetch VM information and deploy load balancers in the project or domain where the nodes reside.
 
 To create the secret, use the following command:
@@ -129,9 +137,17 @@ spec:
 
 **Default:** `"0.0.0.0/0"` (allows all sources)
 
-**Description:** Specifies the source CIDR list for firewall rules on the CloudStack load balancer. This restricts which IP addresses can access the load balancer.
+**Description:** Sets the source CIDR list on the CloudStack **load balancer rule**, restricting the source addresses that the load balancer rule accepts traffic from.
+
+This annotation restricts traffic at the load balancer rule only. It does **not** configure the
+firewall: the firewall rule created alongside the load balancer rule comes from
+`spec.loadBalancerSourceRanges`, which defaults to `0.0.0.0/0`. So if you set only this annotation,
+disallowed sources are still turned away, but by the load balancer instead of being blocked at the
+firewall — see [Restricting Source Traffic](#restricting-source-traffic).
 
-**Use Case:** Use this annotation to restrict access to your load balancer to specific IP ranges for security purposes. This is particularly useful for internal services or when you want to limit access to specific networks.
+**Use Case:** Use this annotation, together with `spec.loadBalancerSourceRanges`, to restrict access to
+your load balancer to specific IP ranges. This is particularly useful for internal services or when
+you want to limit access to specific networks.
 
 **Example:**
 ```yaml
@@ -146,10 +162,124 @@ spec:
 ```
 
 **Format:** Comma-separated list of CIDR ranges. Spaces around commas are automatically trimmed.
+Every entry must parse as a valid CIDR, otherwise the service fails to sync with an `invalid CIDR` error.
+
+**CloudStack Version:** Creating a rule with a CIDR list works on all supported versions.
+*Changing* the CIDR list of an existing rule can only be done in place on CloudStack 4.22 or later.
+On earlier versions the controller deletes the load balancer rule and recreates it with the new CIDR
+list, which briefly interrupts traffic on that port.
+
+**Note:** If the annotation is not set, the load balancer rule allows all sources (`0.0.0.0/0`).
+Setting it to an empty value (`""`) sends an empty CIDR list to CloudStack — it does not block all
+traffic.
+
+#### `service.beta.kubernetes.io/cloudstack-load-balancer-ip-associated-by-controller`
+
+**Type:** Boolean (`"true"` or `"false"`)
+
+**Default:** Not set
+
+**Description:** Set by the controller, not by you. When the controller associates a public IP that
+was not already allocated, it records that fact on the service with this annotation. On deletion the
+annotation determines whether the IP is disassociated again: an IP the controller allocated is
+released, an IP that was already allocated before the service existed is left in place.
 
-**CloudStack Version:** Updating CIDR lists on existing load balancer rules requires CloudStack 4.22 or later. Creating new load balancer rules with CIDR lists works on earlier versions.
+The controller also checks for other load balancer rules on the same IP before releasing it, so an
+IP shared by several services is not disassociated while still in use. Do not set or remove this
+annotation by hand — doing so can leak a public IP or release one that you allocated yourself.
+
+### Restricting Source Traffic
+
+Traffic is filtered at two independent layers, which are configured separately. The second layer is
+either a firewall rule or a Network ACL rule, depending on what the network offers:
+
+| Layer | Configured by | Default |
+| --- | --- | --- |
+| CloudStack load balancer rule | `service.beta.kubernetes.io/cloudstack-load-balancer-source-cidrs` annotation | `0.0.0.0/0` |
+| Firewall rule — isolated networks, and VPC networks that offer the Firewall service | `spec.loadBalancerSourceRanges` | `0.0.0.0/0` |
+| Network ACL rule — VPC networks without the Firewall service | Not configurable | `0.0.0.0/0` |
+
+Traffic has to be allowed by both layers, so where firewall rules are used, either setting alone is
+enough to block unwanted sources. Setting both keeps the two rules consistent in CloudStack.
+Where Network ACL rules are used, `spec.loadBalancerSourceRanges` has no effect and the annotation
+is the only way to restrict sources.
+
+The two layers turn traffic away differently. The firewall discards the packets on the virtual
+router, so a blocked client simply times out. The load balancer rule lets the connection be
+established first and then closes it, so a blocked client can still tell that the port is open.
+Use `spec.loadBalancerSourceRanges` if you would rather not expose that.
+
+To restrict access at both layers, set both:
+
+```yaml
+apiVersion: v1
+kind: Service
+metadata:
+  name: my-service
+  annotations:
+    service.beta.kubernetes.io/cloudstack-load-balancer-source-cidrs: "10.0.0.0/8"
+spec:
+  type: LoadBalancer
+  loadBalancerSourceRanges:
+    - 10.0.0.0/8
+  ports:
+    - port: 80
+      protocol: TCP
+```
+
+The controller never opens the firewall implicitly; it always creates explicit firewall rules for
+the ports it manages, and it removes firewall rules whose CIDR list no longer matches.
+
+### Assigning a Specific IP Address
+
+Set `spec.loadBalancerIP` to pin the load balancer to a known public IP:
+
+```yaml
+spec:
+  type: LoadBalancer
+  loadBalancerIP: 10.1.1.218
+```
 
-**Note:** If the annotation is not set, the default behavior is to allow all sources (`0.0.0.0/0`). However, if you explicitly set the annotation to an empty value (`""`), this will result in an empty CIDR list, effectively blocking all traffic.
+The address must be an existing public IP address visible to the configured account, otherwise the
+service fails to sync with `could not find IP address`. It does not need to be associated with the
+network beforehand: if the address is free, the controller associates it (with the VPC instead of
+the network if the network belongs to a VPC).
+
+When the service is deleted, the IP is released only if the controller associated it — see
+[`...-ip-associated-by-controller`](#servicebetakubernetesiocloudstack-load-balancer-ip-associated-by-controller)
+above.
+
+### Session Affinity
+
+The load balancer algorithm is derived from the service's `spec.sessionAffinity`; there is no
+annotation for it.
+
+| `spec.sessionAffinity` | CloudStack algorithm |
+| --- | --- |
+| `None` (default) | `roundrobin` |
+| `ClientIP` | `source` |
+
+Any other value makes the service fail to sync with `unsupported load balancer affinity`. Other
+CloudStack algorithms, such as `leastconn`, cannot currently be selected.
+
+### VPC Networks
+
+VPC networks are supported. VPC networks normally do not offer the Firewall service, so the
+controller creates **Network ACL** rules instead of firewall rules for the managed ports, and
+associates public IPs with the VPC rather than with the network.
+
+CloudStack 4.23 adds support for firewall rules on public IPs in VPC networks. This requires a
+network offering that includes the Firewall service and is not enabled by default. The controller
+chooses the mechanism based on the services the network offers, so on such networks it manages
+firewall rules instead of Network ACL rules, and `spec.loadBalancerSourceRanges` is applied to
+them.
+
+Two things to be aware of when the controller manages Network ACL rules:
+* The ACL rules the controller creates always allow `0.0.0.0/0`; `spec.loadBalancerSourceRanges` is
+  not applied. Use the `cloudstack-load-balancer-source-cidrs` annotation to restrict sources.
+* If the network uses one of the default ACL lists (`default_allow` or `default_deny`), the
+  controller does not add ACL rules to it. Use a custom ACL list if you want the controller to
+  manage the rules.
 
 ### Node Labels
 
@@ -181,6 +311,73 @@ It is also possible to trigger this process manually by issuing the following co
 kubectl taint nodes <my-node-without-labels> node.cloudprovider.kubernetes.io/uninitialized=true:NoSchedule
 ```
 
+Along with the labels, initialization also sets the node's provider ID, in the form
+`external-cloudstack://<instance UUID>`.
+
+## FAQ
+
+### How do I stop the controller from managing my LoadBalancer services?
+
+Some clusters run on CloudStack but use a different load balancer implementation, and only want the
+node and node lifecycle controllers. There are two ways to do this.
+
+**Per service**, set `spec.loadBalancerClass` to the class handled by your own implementation. The
+upstream service controller ignores any service that has a load balancer class set, so this
+controller never sees it:
+
+```yaml
+spec:
+  type: LoadBalancer
+  loadBalancerClass: example.com/my-own-lb
+```
+
+:warning: `spec.loadBalancerClass` is immutable. Existing services have to be deleted and recreated
+to adopt it, so this is best suited to a cluster you are still building out.
+
+**For the whole cluster**, drop the service controller from the controller manager:
+
+```yaml
+args:
+- --leader-elect=true
+- --cloud-provider=external-cloudstack
+- --cloud-config=/config/cloud-config
+- --controllers=*,-service
+```
+
+The `*` is required: `--controllers` replaces the default list instead of adding to it, so
+`--controllers=-service` on its own disables *every* controller. With `--controllers=*,-service`
+the log should show only `"service" is disabled` on startup.
+
+### Does this provide persistent volumes?
+
+No. This controller manages nodes and load balancers only. For volumes, use a CloudStack CSI driver.
+
+### Does it support Cluster API?
+
+Not directly. Cluster API support for CloudStack lives in
+[cluster-api-provider-cloudstack](https://github.com/kubernetes-sigs/cluster-api-provider-cloudstack).
+
+## Troubleshooting
+
+### Services stay in `<pending>`, log shows `could not find network`
+
+The controller works out which network to create the rules in from the nodes: it matches the node
+names against the CloudStack instance names and takes the network of the instance's first NIC. This
+error means that network could not be read back with the configured credentials. Check that
+`project-id` in the `cloud-config` matches the project the nodes are in, and that the account owning
+the API key can see that network.
+
+### Services stay in `<pending>`, log shows `There are no available nodes for LoadBalancer`
+
+No node passed the service controller's filter. A node is skipped if it is not `Ready`, if it
+carries the `node.kubernetes.io/exclude-from-external-load-balancers` label, or if the cluster
+autoscaler has marked it for deletion.
+
+### `exec /app/cloudstack-ccm: exec format error`
+
+The image architecture does not match the node. Releases up to v1.1.0 were published for amd64 only;
+use a newer release, which ships multi-architecture images including arm64.
+
 ## Migration Guide
 
 There are several notable differences to the old Kubernetes CloudStack cloud provider that need to be taken into
@@ -218,7 +415,7 @@ make
 ```
 
 To build the cloudstack-cloud-controller-manager container, please use the provided Dockerfile.
-The Makefile will also with that and properly tag the resulting container.
+The Makefile will also do that and properly tag the resulting container.
 
 ```bash
 make docker
@@ -234,7 +431,8 @@ so you can simply point it to that.
 ./cloudstack-ccm --cloud-provider external-cloudstack --cloud-config ./cloud-config --kubeconfig ~/.kube/config
 ```
 
-Replace k8s-apiserver with the host name of your Kubernetes development clusters's API server.
+Point `--kubeconfig` at a kubeconfig for your Kubernetes development cluster, and `--cloud-config` at
+a `cloud-config` for the CloudStack installation you want to talk to.
 
 If you don't have a 'real' CloudStack installation, you can also launch a local [simulator instance](https://hub.docker.com/r/cloudstack/simulator) instead. This is very useful for dry-run testing.