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

Add more tests to the subgraph we created to run the tests. #1342

Merged
merged 3 commits into from Aug 13, 2018

Conversation

jlewi
Copy link
Contributor

@jlewi jlewi commented Aug 10, 2018

  • Update wait_for_deployment.py to use KUBECONFIG so its not limited to GKE.

  • I don't think we actually need wait_for_deployment; all it does is wait
    for the CRD to be created and that's not very useful. It would
    be better to wait for the actual controller deployments to start.

  • Create a script wait_for_kubeflow based on deploy_kubeflow.py that
    waits for Kubeflow to be deployed and performs basic checks like
    ensuring everything started correctly.

  • Fix a typo in wait_for_kubeflow

  • Add steps to copy artifacts to prow bucket.

  • Create a workflow for unittests."

  • Don't run workflows.jsonnet on presubmit; we use kfctl_test now.

  • Add an option to kfctl_test to not delete the cluster; useful
    for leaving it up to debug tests.

Related to: #1325 Make our E2E tests more reusable and composable.


This change is Reviewable

@jlewi
Copy link
Contributor Author

jlewi commented Aug 10, 2018

/test all

@jlewi
Copy link
Contributor Author

jlewi commented Aug 10, 2018

Test failures look like SSL errors on checkout.

@jlewi
Copy link
Contributor Author

jlewi commented Aug 10, 2018

/test all

@jlewi
Copy link
Contributor Author

jlewi commented Aug 11, 2018

/test all

@jlewi
Copy link
Contributor Author

jlewi commented Aug 11, 2018

* Update wait_for_deployment.py to use KUBECONFIG so its not limited to GKE.

* I don't think we actually need wait_for_deployment; all it does is wait
  for the CRD to be created and that's not very useful. It would
  be better to wait for the actual controller deployments to start.

* Create a script wait_for_kubeflow based on deploy_kubeflow.py that
  waits for Kubeflow to be deployed and performs basic checks like
  ensuring everything started correctly.

* Fix a typo in wait_for_kubeflow
* Add steps to copy artifacts to prow bucket.
* Create a workflow for unittests."

* Don't run workflows.jsonnet on presubmit; we use kfctl_test now.

* Add an option to kfctl_test to not delete the cluster; useful
  for leaving it up to debug tests.
@jlewi
Copy link
Contributor Author

jlewi commented Aug 11, 2018

/test all

@jlewi jlewi changed the title [wip] Add more tests to the subgraph we created to run the tests. Add more tests to the subgraph we created to run the tests. Aug 12, 2018
@jlewi
Copy link
Contributor Author

jlewi commented Aug 12, 2018

/assign @lluunn

@lluunn
Copy link
Contributor

lluunn commented Aug 13, 2018

will review it tomorrow morning

Copy link
Contributor

@lluunn lluunn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 7 of 9 files at r1.
Reviewable status: 7 of 9 files reviewed, 4 unresolved discussions (waiting on @jlewi, @pdmack, and @gaocegege)


testing/wait_for_deployment.py, line 17 at r1 (raw file):

# limitations under the License.

"""Wait for kubeflow deployment.

[optional] might be better to change the file name to wait_for_kf_deployment since deployment can be confused with k8s resource


testing/workflows/components/kfctl_test.jsonnet, line 138 at r1 (raw file):

  },  // checkout
  {
    template: buildTemplate("create-pr-symlink", [

pr-symlink should be common and can move to workflow.lib?


testing/workflows/components/unit_tests.jsonnet, line 15 at r1 (raw file):

local bucket = params.bucket;

// mountPath is the directory where the volume to store the test data

[optional] The code block (setting up path and dir variables) is also dup in all workflows, might be good to find a way to dedup.


testing/workflows/components/unit_tests.jsonnet, line 54 at r1 (raw file):

    image: image,
    workingDir: working_dir,
    // TODO(jlewi): Change to IfNotPresent.

when should this happen?

Copy link
Contributor Author

@jlewi jlewi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 7 of 9 files reviewed, 4 unresolved discussions (waiting on @jlewi, @pdmack, and @gaocegege)


testing/wait_for_deployment.py, line 17 at r1 (raw file):

Previously, lluunn (Lun-Kai Hsu) wrote…

[optional] might be better to change the file name to wait_for_kf_deployment since deployment can be confused with k8s resource

I think we can eventually get rid of this script but this PR is already quite large and I don't want to cause unrelated breakages.


testing/workflows/components/kfctl_test.jsonnet, line 138 at r1 (raw file):

Previously, lluunn (Lun-Kai Hsu) wrote…

pr-symlink should be common and can move to workflow.lib?

Probably but lets leave that for a follow on PR. It depends on the build template which means we'd have to define a stanza like kfTests in workflows.libsonnet; so its a bit of work and I'd like to leave that to the future.


testing/workflows/components/unit_tests.jsonnet, line 15 at r1 (raw file):

Previously, lluunn (Lun-Kai Hsu) wrote…

[optional] The code block (setting up path and dir variables) is also dup in all workflows, might be good to find a way to dedup.

Its part of the step template; we could try to define a common step template but I think doing that in a way that
makes it easy to override the parts we care about without creating too many layers of obfuscation takes some work.


testing/workflows/components/unit_tests.jsonnet, line 54 at r1 (raw file):

Previously, lluunn (Lun-Kai Hsu) wrote…

when should this happen?

This isn't blocked on anything but I'm hesitant to make more changes in this PR.

@lluunn
Copy link
Contributor

lluunn commented Aug 13, 2018

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lluunn

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 67489b5 into kubeflow:master Aug 13, 2018
saffaalvi pushed a commit to StatCan/kubeflow that referenced this pull request Feb 11, 2021
…#1342)

* Add more tests to the subgraph we created to run the tests.

* Update wait_for_deployment.py to use KUBECONFIG so its not limited to GKE.

* I don't think we actually need wait_for_deployment; all it does is wait
  for the CRD to be created and that's not very useful. It would
  be better to wait for the actual controller deployments to start.

* Create a script wait_for_kubeflow based on deploy_kubeflow.py that
  waits for Kubeflow to be deployed and performs basic checks like
  ensuring everything started correctly.

* Fix a typo in wait_for_kubeflow
* Add steps to copy artifacts to prow bucket.
* Create a workflow for unittests."

* Don't run workflows.jsonnet on presubmit; we use kfctl_test now.

* Add an option to kfctl_test to not delete the cluster; useful
  for leaving it up to debug tests.

* latest.

* Autoformat.
yanniszark pushed a commit to arrikto/kubeflow that referenced this pull request Feb 15, 2021
* Add MPI Job horovod example

* Add link to dockerimage

* Change mpi example docker hub registry

* Remove istio sidecar annotation
surajkota pushed a commit to surajkota/kubeflow that referenced this pull request Jun 13, 2022
…authorization (kubeflow#1342)

* Add argo to stacks/generic

* Pull pipelines manifest from upstream

* Updated kfp

* Minio v3 manifests

* Rename minio configmap

* Add generic minio install

* Generate new test data

* Mysql kustomize v3 manifest - generic install

* Add mysql gcp pd install

* Generate test data

* Pipelines kustomize v3 manifests

* Add kfp ui virtual service

* Add metadata deployment to stacks/generic

* Use common cluster domain

* Deploy metadata writer

* Add kfp cache server

* Update test data

* Enable KFP multi user mode without istio security

* Fix persistence agent watch namespace

* Fix namespace env for some deployments

* Fix cluster roles and bindings

* fix rename

* Fix pipelines ui role

* Updated kfp to rc2

* simplify pipeline v3 manifest using updated kfp rc2 manifest

* Fix pipeline-install-config

* remove redundant configmap

* update tests

* updated to kfp 1.0.0-rc.3

* Adapt to kfp 1.0rc3 refactoring

* update test snapshots

* fix pull kfp script to detect empty dir

* fix example ref

* update snapshot

* fix gcp pd manifest

* Update stacks ref

* revert alice example to gcp stack

* update snapshot

* fix profile controller iam binding

* Update kfp profile controller can be configured to different images and
istio sidecar

* add missing viewer controller cluster roles

* Use python3 for sync.py

* Revert gcp stack back to use non multi user kfp

* revert unintended changes

* revert upstream changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants