cocreators-ee / project-template

Project template for kick-starting your work the right way
Other
5 stars 7 forks source link

The support for obsolete kube files for components is broken #46

Open joakimnordling opened 4 years ago

joakimnordling commented 4 years ago

There's multiple issues with the support for obsolete kube files for a component.

In the _do_release function found in devops/lib/component.py there's a problem that causes a KeyError in this section:

        for config in self.obsolete_kube_configs:
            path = self.kube_configs[config]
            self._delete_kube_config(ctx, path, dry_run)

It should likely be path = self.obsolete_kube_configs[config].

Even if that is fixed, then another problem is that there's no patching of the namespace and other patching that is in place for the normal kube files.

Additionally the merge/override templates are not taken into account for the obsolete kube files.

Furthermore if you want to delete a whole component, you will hit a ValueError: No kube configs found in path/to/component/kube if you moved all the configs to path/to/component/kube/obsolete.

If you run a release more than once with an obsolete kube file for the component (and you pretend the issue with the namespace would have been resolved), then the second time you would get:

subprocess.CalledProcessError: Command '['kubectl', 'delete', '-f', 'path/to/component/kube/obsolete/obsolete.yaml']' returned non-zero exit status 1.

simply because kubectl will crash on the fact that it could not delete the configmap/deployemnt/etc. etc because it was already deleted by the earlier run.

lietu commented 4 years ago

It seems most of these are easy fixes, and I don't see the issue with the overrides/merges as particularly relevant.

The goal of the "obsolete" system is to delete old stuff, not to fully comprehend how they were set up - meaning that "delete deployment xyz from namespace baz" is enough, it likely does not need to know the environment variables or volume mounts for it.

Namespace patching is likely easy to amend to existing system, and the No kube configs error could first check if there are obsolete ones.

And when deleting it could check if the error seems like it's not particularly relevant, or just ignore the errors.

If these still seem complicated, then we could just change the obsolete system to be a simple list of things to delete if seen.

# envs/<env>/settings.py or similar
OBSOLETE_RESOURCES = [
  {"kind": "Deployment", "name": "my-app"}
]

Which then would be evaluated through some get + delete loop which checks if the resource exists in the namespace and if it does, it deletes it.

joakimnordling commented 4 years ago

I actually like the idea of the list of things to delete if seen. If you're deleting a whole component you don't need to keep the folder hierarchy in place, neither the entry for the component in the /envs//settings.py. It also works nicely in case you're using just override-templates; no need to copy a rendered dummy version of it to the component folder.

I would however suggest making two lists of obsolete resources to delete. One that's run before the release and one that's run after the release. The reason to this is that in some cases the new and old resource would cause a conflict (thus you'd need to remove the old one first). And in many cases deleting the old resource after the release is the safer way that causes less disruption.