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

HubSync v1.0 #1548

Merged
merged 4 commits into from Oct 5, 2018
Merged

HubSync v1.0 #1548

merged 4 commits into from Oct 5, 2018

Conversation

jasonsmithio
Copy link
Contributor

@jasonsmithio jasonsmithio commented Sep 18, 2018

This will sync gcr to dockerhub per #1320 . It functions in this way

  1. runs gcloud to pull down all image repos within gcr.io/kubeflow-images-public
  2. runs a follow-up gcloud command to pull down image tags (latest)
  3. Build cloudbuild.yaml file to authenticate with DockerHub, pull the image from gcr.io/kubeflow-images-public, tag the image, and push it to DockerHub

This is base code and is pre-coded largely just to pull from GCR to DockerHub. Users could modify the code in order to push to their own GCR or DockerHub. keys.yaml is the file where you store your credentials.

I am hoping for a version 2.0 with a CLI. This would break it down into steps so that a user can build and sync what they want. However, for now, this will move all images from gcr.io/kubeflow-images-public to DockerHub


This change is Reviewable

@jasonsmithio
Copy link
Contributor Author

/retest

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: jlewi

If they are not already assigned, you can assign the PR to them by writing /assign @jlewi in a comment when ready.

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

@jasonsmithio
Copy link
Contributor Author

/assign @jlewi

@@ -25,8 +25,17 @@ Finally, we will deploy the application
ks apply default -c weaveflux
```

<<<<<<< HEAD
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like you have some merge conflicts in the file.

@jlewi
Copy link
Contributor

jlewi commented Sep 21, 2018

Please use "yaml" not "yml" as the extension.

Can you reference in the PR description the issue tracking syncing the Docker images?

Can you expand the PR description? What I'd like to understand is what this PR is doing in terms of the overall plan to sync our images to DockerHub.

It looks like this is a script that uses gcloud to get a list of the images and then creates a YAML file to retag those images using GCB?

@jasonsmithio
Copy link
Contributor Author

Hello,

Fixed merge conflicts and yaml and updated comment

/retest

@jlewi
Copy link
Contributor

jlewi commented Sep 25, 2018

Thanks for the really great description. Sorry for the slow reply been busy preparing for the summit and helping with the 0.3 release.

I don't think we need to resolve this as part of this PR, but I thought the primary audience for this tool was the Kubeflow release team to make it easy to sync our images from GCR to DockerHub to support users who can't use GCR. Would we still need a CLI in that case?

Ideally I'd like to automate this. The current thinking is we are using Prow to manage building and pushing our Docker images. Prow has a periodic job which is equivalent to a cron job. So my initial thought is that we would setup a prow periodic job to run this script regularly so that we could sync all our images to DockerHub.

releasing/hubsync/hubsync.py Outdated Show resolved Hide resolved
@jlewi
Copy link
Contributor

jlewi commented Sep 25, 2018

It looks like this would push all of our images; even if they already exist in DockerHub. Is this something we will need to fix later?

@jlewi
Copy link
Contributor

jlewi commented Sep 25, 2018

/test all

Test failure looks like a flake since this PR isn't actually changing any code that would affect kfctl.

@jasonsmithio
Copy link
Contributor Author

@jlewi it should just add an extra image tag in the future.

My thoughts around a CLI are more for the end user. The script as it exists today takes all images and uploads them to another source. It's possible an end user might want to do some form of version control of the images for their own use or only use certain images for compliance or other policy. The idea behind the CLI would be to make each step it's own command so that an end user can only do what the feel they need.

@jlewi
Copy link
Contributor

jlewi commented Sep 26, 2018

Looks like the tests are still failing.

@jasonsmithio
Copy link
Contributor Author

@jlewi is there a particular element I can fix? I am having difficulty determining from the log why it's failing.

@jlewi
Copy link
Contributor

jlewi commented Sep 27, 2018

I'd suggest looking at: https://github.com/kubeflow/testing#debugging-failed-tests

You should try to get the logs for the argo step that is failing to see if there is an error.

/test all

@jasonsmithio
Copy link
Contributor Author

@jlewi I took a deeper look at the logs and couldn't find anything specific. I may need a second pair of eyes to confirm.

@jlewi
Copy link
Contributor

jlewi commented Oct 3, 2018

Here' are the pod logs

version \"1.10.7-gke.1\" is unsupported.","status":"INVALID_ARGUMENT","statusMessage":"Bad

Your GKE deployment is failing because your GKE version is out of date. This is because your code is out of date. You need to sync and then rebase on master to pull in the fix.

@jasonsmithio
Copy link
Contributor Author

Well let's see if this does the trick /retest

new v0.1.2

ClusterIP

hubsync

removed commented code

fixed requirements.txt

fixed and rebased

new v0.1.2

ClusterIP

hubsync

fixed merge error

fixed yaml

README update
@k8s-ci-robot
Copy link
Contributor

@thejaysmith: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
kubeflow-presubmit 6007f35 link /test kubeflow-presubmit

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@jlewi
Copy link
Contributor

jlewi commented Oct 5, 2018

@thejaysmith you've suffered enough.

The test failures are unrelated to this PR since the only code modified is in hubsync which doesn't contain any jsonnet files.

I'll ignore the presubmit failures and merge manually.

@jlewi jlewi merged commit 406f881 into kubeflow:master Oct 5, 2018
@jasonsmithio
Copy link
Contributor Author

Appreciated. I kept running the autoformat_jsonnet.sh but apparently didn't work :-/

saffaalvi pushed a commit to StatCan/kubeflow that referenced this pull request Feb 11, 2021
* fixed and rebased

new v0.1.2

ClusterIP

hubsync

removed commented code

fixed requirements.txt

fixed and rebased

new v0.1.2

ClusterIP

hubsync

fixed merge error

fixed yaml

README update

* commenting

* format

* autoformat
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