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.

Sign up for GitHub

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

Jump to bottom

Add CI to verify hydration works. #50

Merged
merged 5 commits into from
Jun 17, 2020
Merged

Add CI to verify hydration works. #50

merged 5 commits into from
Jun 17, 2020

Conversation

jlewi
Copy link
Contributor

@jlewi jlewi commented Jun 15, 2020 •
edited
Loading

  • Write a golang test to verify we can hydrate the manifests

  • Trigger go tests under prow

  • The name of the stack was changed from GCP to generic so we need to update it.

  • Related to Setup CI against blueprint deployments #42

Add CI to verify hydration works.
f2a9845 
* Write a golang test to verify we can hydrate the manifests

* Trigger go tests under prow

* Related to GoogleCloudPlatform#42
@k8s-ci-robot k8s-ci-robot requested review from Bobgy and rmgogogo June 15, 2020 18:25
@kubeflow-bot
Copy link

This change is Reviewable

@jlewi jlewi changed the title Add CI to verify hydration works. [WIP] Add CI to verify hydration works. Jun 15, 2020
@jlewi
Copy link
Contributor Author

jlewi commented Jun 15, 2020

This isn't ready for review yet; I need to verify that the tests actually run successfully.

Jeremy Lewi added 2 commits June 15, 2020 11:27
@jlewi
Copy link
Contributor Author

jlewi commented Jun 15, 2020

Logs show

INFO|2020-06-15T18:31:07|/src/kubeflow/testing/py/kubeflow/testing/tekton_client.py|58| Parsing ApiException body: {"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"pipelineruns.tekton.dev \"kubeflow-gcp-blueprints-presubmit-go-unittests-50-b1d4fcd-3488-c09d\" not found","reason":"NotFound","details":{"name":"kubeflow-gcp-blueprints-presubmit-go-unittests-50-b1d4fcd-3488-c09d","group":"tekton.dev","kind":"pipelineruns"},"code":404}

I suspect the problem is that the name is too long so it gets rejected.

Name is 67 characters long

@jlewi jlewi mentioned this pull request Jun 15, 2020
Closed
@jlewi
Copy link
Contributor Author

jlewi commented Jun 16, 2020

kubeflow/testing#683 was merged; lets see if that fixes the problem.

/test all

@jlewi
Copy link
Contributor Author

jlewi commented Jun 16, 2020

go tests actually failed even though the presubmit is reported as success; probably because we aren't probably taking the junit files into account in the github status.

@Bobgy
Copy link
Contributor

Bobgy commented Jun 16, 2020

Thanks! This helps development a lot.
/hold
Please remove hold when you feel ready to review (to make the signal clearer)

@jlewi
Copy link
Contributor Author

jlewi commented Jun 16, 2020

/test all

@jlewi jlewi mentioned this pull request Jun 16, 2020
Closed
* Fix the tests by moving go.mod to the root of the repo. It looks
c29cc71 
  like that is what the go unittests task currently requires.
@jlewi
Copy link
Contributor Author

jlewi commented Jun 16, 2020

The test failure

go test ./...
# _/git_gcp-blueprints/tests
package _/git_gcp-blueprints/tests (test)
	imports github.com/termie/go-shutil: cannot find package "github.com/termie/go-shutil" in any of:
	/usr/local/go/src/github.com/termie/go-shutil (from $GOROOT)
	/root/go/src/github.com/termie/go-shutil (from $GOPATH)
FAIL	_/git_gcp-blueprints/tests [setup failed]
FAIL

Is because we are issuing "go test ./..." in the root of the repository which doesn't have a "go.mod" file.

@jlewi jlewi mentioned this pull request Jun 16, 2020
Closed
@jlewi jlewi changed the title [WIP] Add CI to verify hydration works. Add CI to verify hydration works. Jun 16, 2020
@jlewi
Copy link
Contributor Author

jlewi commented Jun 16, 2020

/hold cancel

@Bobgy Tests are passing; this is ready for review.

Bobgy
Bobgy reviewed Jun 17, 2020
View reviewed changes
Copy link
Contributor

@Bobgy Bobgy left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve
Thanks @jlewi! this is great for development

kubeflow/instance/kustomize/kubeflow-apps/kustomization.yaml
@@ -5,5 +5,5 @@ patchesStrategicMerge:
- default-install-config.yaml
- pipeline-minio-install-config.yaml
resources:
- ../../../upstream/manifests/stacks/gcp # {"type":"string","x-kustomize":{"setBy":"kpt","partialSetters":[{"name":"kustomize_manifests_path","value":"../../../upstream/manifests"}]}}
Copy link
Contributor

Choose a reason for hiding this comment

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

not very related to this PR, but shall we add gcp stacks back to manifests, I'm making some gcp specific components and I think a gcp stacks is needed, we cannot compose all of them in instance/ folder

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Bobgy

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 3958738 into GoogleCloudPlatform:master Jun 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Bobgy Bobgy Bobgy left review comments

@rmgogogo rmgogogo Awaiting requested review from rmgogogo

Assignees

@Bobgy Bobgy

Labels
approved lgtm size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jlewi @kubeflow-bot @Bobgy @k8s-ci-robot