istio / istio.io

Source for the istio.io site
https://istio.io/
Apache License 2.0
761 stars 1.54k forks source link

Installation Options page doesn't describe global.meshNetworks #3739

Closed esnible closed 5 years ago

esnible commented 5 years ago

The page https://preliminary.istio.io/docs/reference/config/installation-options/ just says the default for meshNetworks is {} but doesn't say what it should be.

https://preliminary.istio.io/docs/examples/multicluster/split-horizon-eds/ shows an example of setting

 --set global.meshExpansion.enabled=true \
  --set global.meshNetworks.network2.endpoints[0].fromRegistry=n2-k8s-config \
  --set global.meshNetworks.network2.gateways[0].address=0.0.0.0 \
  --set global.meshNetworks.network2.gateways[0].port=443 \

Almost certainly we need to document meshNetworks. It is unclear if the user is supposed supply JSON and what it should look like.

This is related to https://github.com/istio/istio.io/issues/1936

johnma14 commented 5 years ago

@esnible There is a way to print the documentation here: https://github.com/istio/istio/blob/master/install/kubernetes/helm/istio/values.yaml#L411 for meshNetworks, but the reason I do not print it now is because of this case: https://github.com/istio/istio/blob/master/install/kubernetes/helm/istio/charts/grafana/values.yaml#L18 (where according to ruamel, the comments for persist: false is the commented out text above that - although those are just commented out properties). One thing I can do is go ahead and process the descriptions like this and cleanup the grafana and other values.yaml file so that we do not have any commented out properties like that. Any thoughts?

esnible commented 5 years ago

I don't understand how the documentation is generated from values.yaml.

Is it possible to have two kinds of comments, # and ##, and one goes to documentation and one doesn't?

Is it possible to have a comment for value that is something like # see https://istio.io/help/ops/component-logging/#controlling-output

Many of the installation options don't have descriptions. In many cases that is OK; if a key ends with ".enabled" it should be obvious what the key does. There are a few keys on https://preliminary.istio.io/docs/reference/config/installation-options/ that need documentation. These are the ones I'd like to prioritize:

Keys with no description whose default is {}: certmanager.resources, gateways.istio-ingressgateway.resource, everything .serviceAnnotations, everything .podAnnotations, everything .nodeSelector, global.meshNetworks,

Keys with no description whose default is []: gateways.istio-ingressgateway.loadBalancerSourceRanges, gateways.istio-ingressgateway.externalIPs,

Keys with no description whose default is "" and I have no idea: global.sds.udsPath,

Keys that are probably enumerations but the other values aren't given: grafana.dashboardProviders.dashboardproviders.providers.type, tracing.provider

johnma14 commented 5 years ago

@esnible @geeknoid @sdake @linsun the script that parses through the values.yaml files uses ruamel to parse yaml and get the description for each configuration option. To give a very high level view of how it works - as long as user provides documentation in this format:

we can process the documentation for it without any problem. The option global.meshNetworks has documentation that fits this pattern. For example:

# Configure the mesh networks to be used by the Split Horizon EDS.
  #
  # The following example defines two networks with different endpoints association methods.
  # For `network1` all endpoints that their IP belongs to the provided CIDR range will be
  # mapped to network1. The gateway for this network example is specified by its public IP
  # address and port.
  # The second network, `network2`, in this example is defined differently with all endpoints
  # retrieved through the specified Multi-Cluster registry being mapped to network2. The
  # gateway is also defined differently with the name of the gateway service on the remote
  # cluster. The public IP for the gateway will be determined from that remote service (not
  # supported yet).
  #
  # meshNetworks:
  #   network1:
  #     endpoints:
#     - fromCidr: "192.168.0.1/24"

and I can read this one but the reason I ignore this is because of the fact that I saw in other values.yaml files where these key-value pairs were commented out (not because it was part of a documentation but simply because it was either not needed or not valid). Here are couple of examples:

will appear as comments for persist: false, which is not correct.

One way to fix this is to go through all the values.yaml files under the charts directory and clean it to get rid of the commented key-value pairs that are not part of the documentation. That way we can capture the valid documentation correctly. Looking forward to hearing your thoughts.

linsun commented 5 years ago

@johnma14 I'd vote for this -- One way to fix this is to go through all the values.yaml files under the charts directory and clean it to get rid of the commented key-value pairs that are not part of the documentation.

johnma14 commented 5 years ago

Closing this one for now. Opened this issue: https://github.com/istio/istio.io/issues/4497 to track insufficient documentation for all configuration options. Thanks Ed.