cloudfoundry-incubator / kubecf

Cloud Foundry on Kubernetes
Apache License 2.0
115 stars 62 forks source link

fix: chart: Axe tcp-router-public if unavailable #1559

Closed mook-as closed 4 years ago

mook-as commented 4 years ago

Description

If routing-api is not enabled, then remove the tcp-router-public service (when not using ingress). Otherwise we end up with a service that will never point at anything.

Motivation and Context

Discovered while working on #1144. We will never finish waiting for services to be ready if we have a service that can never be ready.

How Has This Been Tested?

Tested locally (and in the WIP CI stuff); I can actually finish waiting now.

Types of changes

Checklist:

viccuad commented 4 years ago

Hm. Concourse CI fails on the gcloud DNS transaction, as there's no tcp-router-ip for the service: https://concourse.suse.dev/builds/40662#L5fabb78f:615 https://github.com/cloudfoundry-incubator/kubecf/blob/aec0b2e01b68e54f54a62b756457a3aafbfae88f/.concourse/pipeline.yaml.gomplate#L240-L241

Concourse CI doesn't use Catapult nor external-dns for the creation of GKE clusters.

To not overcomplicate things for now, I have committed a valid but bogus value to $tcp_router_ip on the pipeline, when the service is not present. The correct way would be to either use external-dns properly (moving the pipeline code to Catapult) or read features.routing_api which is not there until later. But those imply big refactoring of the Concourse pipeline, and we are moving to GH Actions.

viccuad commented 4 years ago

Just realized that for testing this PR in Concourse, either we fly the pipeline from this branch, or deploy a copy of the pipeline with config including:

github_status: true
branches: # Repository branches to track
- marky/service-no-tcp
pr_resources:
- pr
pr_base_branch: marky/service-no-tcp # filter PRs depending on the branch they target. `~` for all branches

Tried the second one, but flying failed with: error: yaml: line 427: did not find expected alphabetic or numeric character. Looking into it I think that the pr resource doesn't support / chars. Woould be weird. Will continue tomorrow.

viccuad commented 4 years ago

I have flown the Concourse changes to the kubecf pipeline, the PR should pass now.

viccuad commented 4 years ago

Reflown changes to pipeline, other PRs went through with them, so they are validated (e.g: https://github.com/cloudfoundry-incubator/kubecf/pull/1593). Yes, that it's reckless..