Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Manifests apply order #2645

Closed
tomtaz opened this issue Aug 14, 2019 · 4 comments · Fixed by #2729
Closed

Manifests apply order #2645

tomtaz opened this issue Aug 14, 2019 · 4 comments · Fixed by #2729

Comments

@tomtaz
Copy link

tomtaz commented Aug 14, 2019

Manifests apply order

Expected behavior

Skaffold should apply the manifests in the order they are supplied in the manifests property.
(see @briandealwis below for use case/context)

Actual behavior

Uses (apparently) alphabetical order for everything.

Information

  • Skaffold version: v0.35.0
  • Operating system: osx

Steps to reproduce the behavior

apiVersion: skaffold/v1beta13
kind: Config
deploy:
  kubectl:
    manifests:
      - ../folder_a/*
      - ./some_other_manifest.yaml

Whatever the order the manifests is in the manifests property, skaffold dev -v debug still shows they are applied in the same order.

@briandealwis
Copy link
Member

briandealwis commented Aug 14, 2019

Relevance here is that @thomas-tabbaza is trying to create and use namespaces within his manifests, and is experiencing failures:

Generating tags...
Tags generated in 27.396µs
Starting deploy...
kubectl client version: 1.14
namespace/kinow-platform created
Error from server (NotFound): error when creating "STDIN": namespaces "kinow-platform" not found
Error from server (NotFound): error when creating "STDIN": namespaces "kinow-platform" not found
Cleaning up...
namespace "kinow-platform" deleted
Cleanup complete in 5.373936214s

Namespaces are required to be ordered to the top:

create the namespace before running apply, or sort your input files to put hard dependencies first.

there are very few hard dependencies in kube, but the namespace existing is one of them

There is some more discussion in kubernetes/kubernetes#16448. Skaffold's kubectl deployer should retain the manifest order specified.

Apparently kustomize sorts resources and so using the kustomize deployer may be a workaround.

@balopat
Copy link
Contributor

balopat commented Aug 14, 2019

So, we create a single big list of manifests out of the manifest list and throw that at kubectl apply and kubectl doesn't sort resources.
Options:

  1. instead of a single monolithic apply, apply per manifest entry - can make things significantly slower on remote clusters
  2. sort the manifests - reimplementing the wheel - also requires parsing the manifests. It's doable but not a small change.
  3. turn on -k if the user has kubectl 1.14+ installed (<1.14 doesn't work)

I'm currently thinking we could do 3) and 2) for <1.14 and retire 2) when <1.14 kubectl versions are not supported (https://kubernetes.io/docs/setup/release/version-skew-policy/) ...

@tomtaz
Copy link
Author

tomtaz commented Aug 14, 2019

Just passing the manifest files in the order they are listed in the manifests: property should do the trick without downsides. Then we can just advise people to put namespaces first.

Example:

apiVersion: skaffold/v1beta13
kind: Config
deploy:
  kubectl:
    manifests:
      # 1st file in the apply command
      - traefik-ingress.yaml
      # 2nd file in the apply command
      - k8s-dashboard-ingress.yaml  

Currently, this is not the case, k8s-dashboard-ingress.yaml would be first (alphabetical order). When referencing folders though (- folder/*) it should stay alphabetical, since it is the expected behaviour.

@balopat
Copy link
Contributor

balopat commented Aug 14, 2019

also related to #1781

@dgageot dgageot self-assigned this Aug 27, 2019
dgageot added a commit to dgageot/skaffold that referenced this issue Aug 27, 2019
Fix GoogleContainerTools#2645

Signed-off-by: David Gageot <david@gageot.net>
dgageot added a commit to dgageot/skaffold that referenced this issue Aug 27, 2019
Fix GoogleContainerTools#2645

Signed-off-by: David Gageot <david@gageot.net>
dgageot added a commit to dgageot/skaffold that referenced this issue Aug 28, 2019
Fix GoogleContainerTools#2645

Signed-off-by: David Gageot <david@gageot.net>
dgageot added a commit to dgageot/skaffold that referenced this issue Aug 28, 2019
Fix GoogleContainerTools#2645

Signed-off-by: David Gageot <david@gageot.net>
dgageot added a commit to dgageot/skaffold that referenced this issue Aug 28, 2019
Fix GoogleContainerTools#2645

Signed-off-by: David Gageot <david@gageot.net>
dgageot added a commit that referenced this issue Aug 28, 2019
* List manifests in the order given by the user

Fix #2645

Signed-off-by: David Gageot <david@gageot.net>

* s/list/set/ 

Signed-off-by: David Gageot <david@gageot.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants