kops tries to delete route tables and nat gateways when subnets are shared

[valdisrigdon]

Thanks for submitting an issue! Please fill in as much of the template below as
you can.

------------- BUG REPORT TEMPLATE --------------------

1. What kops version are you running? The command kops version, will display
   this information.

1.9.0.alpha2

2. What Kubernetes version are you running? kubectl version will print the
   version if a cluster is running or provide the Kubernetes version specified as
   a kops flag.

1.9.6

3. What cloud provider are you using?

aws

4. What commands did you run? What is the simplest way to reproduce this issue?

Create a cluster with a private topology with subnets already created. Destroy the cluster and the route tables and nat gateways that kops did not create are deleted.

5. What happened after the commands executed?

Route tables, EIPs, and NAT gateways are deleted.

6. What did you expect to happen?

kops didn't create them -- don't delete them!

7. Please provide your cluster manifest. Execute
   kops get --name my.example.com -oyaml to display your cluster manifest.
   You may want to remove your cluster name and other sensitive information.

8. Please run the commands with most verbose logging by adding the -v 10 flag.
   Paste the logs into this report, or in a gist and provide the gist link here.

9. Anything else do we need to know?

If I Tag the Routetable with AssociatedNatgateway: nat-id and kubernetes.io.cluster/<cluster-name>: shared, kops doesn't attempt to delete them.

[valdisrigdon]

This maybe related to #4614, but I thought I'd open this incase it isn't related.

I can help contribute a fix if pointed in the right direction.

[chrislovecnm]

Components need to be tagged shared label. When kops first ran did it try to tag the resources?

[chrislovecnm]

There has been internal discussion if kops should or should not delete resources that are not tagged.

@justinsb can you comment.

[valdisrigdon]

When kops first ran, it did not tag the existing route tables, subnets, or nat gateways.

[justinsb]

Thanks for reporting @valdisrigdon and sorry for the problem.

You're absolutely correct - if kops does not create the object, it should not delete it! It does get a little trickier for EIPs because it used to be the case that they could not be tagged, so I'm really looking at route tables.

Was your route-table was actually deleted? Because kops shouldn't do that if the attached subnets are not deleted.

Was kops also trying to delete the subnets?

And you'll see we had a bug in alpha.2 where we might have tried to delete route tables when the VPC was shared - but this should have simply failed if the subnets were shared.

Was the cluster also created with alpha.2? I'm working to reproduce it & add test coverage, but I'm struggling to hit the same problem.

[valdisrigdon]

The cluster was created. The subnets were shared and kops did not try to delete them. It was actively trying to delete route tables. It also followed the route tables and did delete the NAT gateways and the route to the nat gateway. On a delete it also indicated that it found new EIPs and marked those to be deleted as well.

It never actually deleted the route table because of dependencies from other AWS objects I believe.

[chrislovecnm]

@valdisrigdon I fixed the deletion of the route table, and the new PRs this weekend should clear up NGW, eips, etc. We did not have enough tags on enough items. These fixes will be in the alpha or beta. They are in master if you want to test drive them. Should merge shortly.

Thanks to @justinsb

[valdisrigdon]

Thanks for the quick turnaround! I'll test this out today.

Is it expected that I would need to tag each route table or given I'm using shared subnets and shared VPC that kops won't delete the networking stack?

[justinsb]

@valdisrigdon with kops 1.9.0-alpha.3 or beta.1 or later, it should "just work". We'll add a "k8s.io/cluster/=sharedtag and we then won't delete it. We're also now more conservative about deleting potentially shared resources generally - we no longer assume that the lack of a tag means it can be deleted, we look for an owned tag (eitherk8s.io/cluster/clustername=ownedor the legacy tagKubernetesCluster=`)

[valdisrigdon]

My testing was successful as well. Thanks for fixing!

[justinsb]

Great news - thanks for confirming!