canonical / kfp-operators

Kubeflow Pipelines Operators
Apache License 2.0
2 stars 12 forks source link

Minio credentials not working when submitting demo pipeline #50

Closed grobbie closed 2 years ago

grobbie commented 2 years ago

On a brand new setup, I launch a demo pipeline. The first step (coin flip) fails with an error indicating that the Minio username/password is incorrect.

On investigation, it looks to me like the K8s secret containing the credentials is not correctly mapped to the pod running the Argo client executing the workflow step.

...
'f:volumes':
            .: {}
            'k:{"name":"mlpipeline-minio-artifact"}':
              .: {}
              'f:name': {}
              'f:secret':
                .: {}
                'f:defaultMode': {}
                'f:items': {}
                'f:secretName': {}
            'k:{"name":"podmetadata"}':
              .: {}
              'f:downwardAPI':
                .: {}
                'f:defaultMode': {}
                'f:items': {}
              'f:name': {}
...

This means that KFP is unusable which is something of a showstopper.

DnPlas commented 2 years ago

Hi @grobbie, thanks for reporting this issue. Unfortunately I was not able to reproduce the issue with the information provided. I did a couple of tests and this is what I got:

Machine configuration

Test 1

New machine, fresh deployment, LDAP, multiuser, dex with LDAP connector Followed almost the same installation steps in Charmed Kubeflow docs, except for the dex configuration, for that I setup LDAP following this guide. No issues logging in the dashboard, no issues running the example pipeline. NOTE: The failed step you can see in the graph is due to an unrelated error. image

Test 2

New machine, fresh deployment, single user, dex basic configuration Followed the installation steps in Charmed Kubeflow docs. No issues logging in the dashboard, no issues running the example pipeline.

Request

If the issue happened under different conditions than the ones I just described, please let us know so we can try reproducing it again.

grobbie commented 2 years ago

Thanks, I was able to make this work by deleting the user's namespace with:

microk8s.kubectl delete namespace test-user1

and then trying again. Screenshot from 2022-02-04 11-22-13 However, there are two undedrlying issues which I realised was the root cause: 1 I had previously deployed Charmed Kubeflow and changed the Minio access credentials, with juju config minio acess-key AAAA secret-key BBBB; however this rendered Minio unusable by KFP for some reason - this is a bug.

2 I then ran juju destroy-model kubeflow and reinstalled Charmed Kubeflow. However, the user namespace test-user1 was not deleted, and the secret containing the (changed) minio access credentials was left behind. This, it could be argued, is unexpected behaviour. I expected that if I destroyed the model, everything would be gone, including any user namespaces created by Charmed Kubeflow. This needs some careful thought - it probably wouldn't bother me, but that the minio access credentials had been left behind and then not refreshed upon redeployment.

DnPlas commented 2 years ago

I see, I am now able to reproduce the issue. As you pointed out, the namespace of the user is not removed when you destroy the model; therefore secrets like mlpipeline-minio-artifact are not removed/cleaned up properly. We'll have to take a look at the way we cleanup k8s resources.

Steps to reproduce

  1. Deploy Charmed Kubeflow v1.4 in Microk8s 1.21/latest as described here
  2. Change minio access-key and secret-key: juju config minio access-key AAAA secret-key BBBB
  3. Destroy the model juju destroy-model kubeflow --destroy-storage and re-deploy as in step 1
  4. Run the example pipeline, the first step throws the following: This step is in Error state with this message: Error (exit code 1): failed to put file: The request signature we calculated does not match the signature you provided. Check your key and signing method.
ca-scribner commented 2 years ago

This should be fixed by #59