Skip to content

Commit

Permalink
Quick start fixes (#269)
Browse files Browse the repository at this point in the history
* Correct quickstart to properly set armadaUrl

Armada url used to be specified under armada.url

Now it is specified under apiConnection.armadaUrl

* Adding ForceNoTls flag in ApiConnectionDetails

Sometimes the executor needs to be forced to use no TLS
 - Currently only uses no tls when connecting to "localhost"

This allows me to use ForceNoTls in the quickstart, as we aren't connecting to localhost and instead a specific ip address

* Fixing setting forceNoTls in quickstart
  • Loading branch information
JamesMurkin authored Dec 5, 2019
1 parent 11ddeb9 commit e750bc7
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 5 deletions.
4 changes: 2 additions & 2 deletions docs/quickstart.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ helm install stable/prometheus-operator --name=prometheus-operator -f docs/quick
kubectl apply -f docs/quickstart/prometheus-kubemetrics-rules.yaml

# Install executor
helm template ./deployment/executor --set image.tag=$ARMADA_VERSION --set applicationConfig.armada.url="$DOCKERHOSTIP:30000" --set prometheus.enabled=true | kubectl apply -f -
helm template ./deployment/executor --set image.tag=$ARMADA_VERSION --set applicationConfig.apiConnection.armadaUrl="$DOCKERHOSTIP:30000" --set applicationConfig.apiConnection.forceNoTls=true --set prometheus.enabled=true | kubectl apply -f -
```
Second executor:
```bash
Expand All @@ -87,7 +87,7 @@ helm install stable/prometheus-operator --name=prometheus-operator -f docs/quick
kubectl apply -f docs/quickstart/prometheus-kubemetrics-rules.yaml

# Install executor
helm template ./deployment/executor --set image.tag=$ARMADA_VERSION --set applicationConfig.armada.url="$DOCKERHOSTIP:30000" --set prometheus.enabled=true | kubectl apply -f -
helm template ./deployment/executor --set image.tag=$ARMADA_VERSION --set applicationConfig.apiConnection.armadaUrl="$DOCKERHOSTIP:30000" --set applicationConfig.apiConnection.forceNoTls=true --set prometheus.enabled=true | kubectl apply -f -
```
### Grafana configuration

Expand Down
7 changes: 4 additions & 3 deletions internal/common/client/connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ type ApiConnectionDetails struct {
OpenIdPasswordAuth oidc.ClientPasswordDetails
OpenIdClientCredentialsAuth oidc.ClientCredentialsDetails
KerberosAuth kerberos.ClientConfig
ForceNoTls bool
}

func CreateApiConnection(config *ApiConnectionDetails, additionalDialOptions ...grpc.DialOption) (*grpc.ClientConn, error) {
Expand All @@ -37,7 +38,7 @@ func CreateApiConnection(config *ApiConnectionDetails, additionalDialOptions ...
defaultCallOptions,
unuaryInterceptors,
streamInterceptors,
transportCredentials(config.ArmadaUrl))
transportCredentials(config))

creds, err := perRpcCredentials(config)
if err != nil {
Expand Down Expand Up @@ -69,8 +70,8 @@ func perRpcCredentials(config *ApiConnectionDetails) (credentials.PerRPCCredenti
return nil, nil
}

func transportCredentials(url string) grpc.DialOption {
if !strings.Contains(url, "localhost") {
func transportCredentials(config *ApiConnectionDetails) grpc.DialOption {
if !config.ForceNoTls && !strings.Contains(config.ArmadaUrl, "localhost") {
return grpc.WithTransportCredentials(credentials.NewClientTLSFromCert(nil, ""))
}
return grpc.WithInsecure()
Expand Down

0 comments on commit e750bc7

Please sign in to comment.