Let the publishing robot publish k8s.io/apimachinery and k8s.io/client-go #1784

caesarxuchao · 2017-02-03T05:58:37Z

TODO (follow-up):

create labels for publish failure issues.
Let the publishing robot publish k8s.io/apimachinery and k8s.io/client-go #1784 (comment)

The robot first publishes k8s.io/apimachinery, then publishes k8s.io/client-go. In the meantime, the publishing robot lets client-go vendor the just published k8s.io/apimachinery.

We need to merge kubernetes/kubernetes#40909 first.

Sample output:
The commits after "CHAO: starting commit of the test" what the robot published. The robot took k8s.io/kubernetes/staging as the source of truth.

apimachinery:
https://github.com/caesarxuchao/apimachinery/commits/master

client-go master:
https://github.com/caesarxuchao/client-go/commits/master

client-go release-2.0:
https://github.com/caesarxuchao/client-go/commits/release-2.0

@lavalamp @sttts @deads2k @mml

I haven't figured out how to write an effective test for the robot.

deads2k · 2017-02-03T12:43:20Z

mungegithub/mungers/publish_scripts/apimachinery_sync_from_kubernetes.sh

+previousKubeSHA=$(cat kubernetes-sha)
+previousBranchSHA=$(cat filter-branch-sha)
+
+# hack...


What's this? Were the commits still wrong somehow?

Please add a comment why this hack is here and when it can go.

I'll remove those before merging. Because my test environment was setup at that point, so the hack it needed for my local experiment.

deads2k · 2017-02-03T12:45:08Z

mungegithub/mungers/publish_scripts/apimachinery_sync_from_kubernetes.sh

@@ -0,0 +1,90 @@
+#!/bin/bash


Was there no reasonable way to get the sync script from the repo? In openshift we ran two repos for a long time (still do for some things), but it does make it harder to describe where people can find, modify, and test changes.

We can get the script from the repo, but I think it's better to centralize all the publishing logic in test-infra. If a developer is going to change the publishing logic, probably he'll need to update the scripts and other parts of the robot at the same time. The workflow is much easier if all the pieces are in the same repo.

We can get the script from the repo, but I think it's better to centralize all the publishing logic in test-infra. If a developer is going to change the publishing logic, probably he'll need to update the scripts and other parts of the robot at the same time. The workflow is much easier if all the pieces are in the same repo.

Thing is, no one but a googler can actually check on this script. That really limits the pool of people who can contribute.

We're slipping pretty far out of date (need a sync pretty badly) so I wouldn't block on it, but I think this will make it harder to make further improvements.

I don't follow the argument. I think what would impede contribution is that the cluster is running in a google provided cluster, not the location of the scripts.

agree w/ Chao

deads2k · 2017-02-03T12:46:17Z

mungegithub/mungers/publisher.go

@@ -46,6 +49,10 @@ func (c coordinate) String() string {
 type repoRules struct {


Is there someone more familiar with what these pieces do?

@mml is familiar with this file. @mml could you help? Thanks.

deads2k · 2017-02-03T12:48:38Z

I think @sttts moved the last of the genericapiserver packages last night, but we'll have to publish the chain to allow the dependent projects to godep them. Will it mess things up to manually publish again? Do you think this is close enough we can just wait?

sttts · 2017-02-03T14:19:23Z

mungegithub/mungers/publish_scripts/apimachinery_sync_from_kubernetes.sh

+git config --global user.email "[email protected]"
+git config --global user.name "Kubernetes Publisher"
+
+dir=$(mktemp -d "${TMPDIR:-/tmp/}$(basename 0).XXXXXXXXXXXX")


basename $0

sttts · 2017-02-03T14:21:14Z

mungegithub/mungers/publish_scripts/apimachinery_sync_from_kubernetes.sh

+git fetch upstream-kube
+
+currBranch=$(git rev-parse --abbrev-ref HEAD)
+previousKubeSHA=$(cat kubernetes-sha)


Would be nice annotate the client-go commits with the kube counterpart commit. Not sure this is easy to do.

X-Kubernetes-Commit: 9483205802394850293452134234

There is git filter-branch --msg-filter .... for that.

I'll integrate this suggestion later. I don't want to spend too much time on this, sorry.

follow-up is fine.

sttts · 2017-02-03T14:22:41Z

mungegithub/mungers/publish_scripts/apimachinery_sync_from_kubernetes.sh

+fi
+
+git branch -D kube-sync || true
+git checkout upstream-kube/master -b kube-sync


what about releases? 1.6, 1.7....

what about releases? 1.6, 1.7....

Env var I'd think.

Releases can be set up in the repoRules in publisher.go and can be passed to this script.

@deads2k

Automatic merge from submit-queue (batch tested with PRs 40862, 40909) Remove apimachinery from staging client-go/Godeps/Godeps.json The publishing robot will add the latest version of apimachinery to Godeps.json. This is part of the effort to allow update staging apimachinery and staging client-go in a same PR. The robot change is here: kubernetes/test-infra#1784 @deads2k @stts @lavalamp

deads2k · 2017-02-03T17:49:40Z

@lavalamp were you familiar with this before? neither sttts nor myself can actually see previous runs or actually merge this.

sttts · 2017-02-03T19:50:02Z

Was chatting with @caesarxuchao earlier today about seeing the logs. The bot pod runs on some google internal GKE instance. To make the logs visible we could use some GCE cloud storage. Maybe it would be much more elegant though to integrate with Github instead and create an issue if the merge breaks (appending new comments, if it breaks again the next day). @caesarxuchao wants to look into this tomorrow or on monday.

lavalamp · 2017-02-03T23:05:08Z

mungegithub/Dockerfile-publisher

+ENV PATH="/usr/local/go/bin:${PATH}"
+
+ENV GOPATH=/
+RUN go get github.com/tools/godep


pin to version, maybe?

Done. Pinned to v75.

We have v79, v75, v74 pinned "somewhere".

caesarxuchao · 2017-02-06T14:42:29Z

@sttts, the last commit added the ability to create an issue if error occurs during a publisher run. Here are a few things I want to discuss:

Shall the issue be created in k8s.io/kubernetes or k8s.io/test-infra, currently it's the former.
I couldn't find an easy way to extract the log starting from the last run. Currently I just print the last 15,000 bytes from the log file. Do you have any suggestion?

@foxish could you help review the last commit? I think you are familiar with the IssueCacher and IssueSyncer. Thanks.

sttts · 2017-02-06T22:22:13Z

mungegithub/mungers/publisher.go

+		glog.Flush()
+		// maxLogLength is the estimated number of characters of the log created
+		// in each run of the publisher
+		var maxLogLength = int64(15000)


Alternatively, we could have some marker line and search for that. Or is there some log rotation in glog?

Yeah, I thought of the marker as well, I'll try it.

glog has rotation, when the log file size exceeds 1800MB. But there's no public API to manually trigger it.

sttts · 2017-02-06T22:25:25Z

@caesarxuchao both kubernetes or test-infra would be fine. I tend to the later.

Can you pass a list of @xyz like github accounts which are cc'ed to the issue?

caesarxuchao · 2017-02-07T03:44:11Z

Can you pass a list of @xyz like github accounts which are cc'ed to the issue?

Yeah, we can. Whom to include? Starts with you, me, deads2k, lavalamp?

sttts · 2017-02-07T09:16:05Z

@caesarxuchao Make a github group out of and add the 4 of us there. @kubernetes/kubernetes-staging-publish-cops

caesarxuchao · 2017-02-07T15:44:27Z

@foxish, the issue-cache is never synced. I think I missed some pieces. Could you help take a look at the third commit? Thanks.

k8s-reviewable · 2017-02-07T15:45:37Z

This change is

foxish · 2017-02-08T05:41:00Z

@caesarxuchao, if you're trying to create comments, you may want to reuse the pattern that the approval-munger uses, such as here. Adding @grodrigues3 and @apelisse who wrote a lot of the new stuff with regard to adding/deleting comments.

caesarxuchao · 2017-02-08T08:57:59Z

@foxish No, it's not about creating comments. I need the robot to create an issue or update the issue if it's not closed. I think issue-syncer and issue-cacher are the right module to use.

caesarxuchao · 2017-02-08T14:42:31Z

update: need a little more time tmr to fix the third commit.

caesarxuchao · 2017-02-09T09:01:38Z

@sttts I met some problems when trying to export the log file created by glog. Although I passed --log_dir to set the default location of the log file, the log still ends up in a random file in the /tmp dir. This is because flags are parsed in main(), but the first invocation of glog is in init(), and upon its first invocation, glog creates the log file. I'm looking for a workaround.

sttts · 2017-02-09T09:03:24Z

If nothing helps, move flags parsing into the init func. Not nice, but might work.

lavalamp · 2017-02-09T23:44:22Z

mungegithub/mungers/issue-cacher.go

@@ -70,6 +70,7 @@ func (p *IssueCacher) RequiredFeatures() []string { return []string{} }

 // Initialize will initialize the munger
 func (p *IssueCacher) Initialize(config *github.Config, features *features.Features) error {
+	// TODO: this need to be changed


Comments like this are more useful if they state what about it needs to be changed :)

Sorry, I forgot to remove this comment I left during debugging.

lavalamp · 2017-02-09T23:46:23Z

mungegithub/mungers/publish_scripts/clientgo_publish.sh

@@ -43,6 +44,16 @@ if git diff --cached --exit-code &>/dev/null; then
    exit 0
 fi
 git commit -m "${MESSAGE}"
+


Please add a comment describing why the next section is needed.

lavalamp · 2017-02-09T23:48:04Z

mungegithub/mungers/publish_scripts/clientgo_publish.sh

+if git diff --cached --exit-code &>/dev/null; then
+    echo "dependency has not changed!"
+else
+    git commit -m "update dependency, should only contain changes in k8s.io/apimachinery"


"Pick up new dependencies"?

I don't recommend including references to specific packages when it looks like the above could have updated lots of stuff.

The staging area should have the latest dependencies, except for the k8s repos, like apimachinery.

I'll rephrase to "Pick up new dependencies on other k8s repos".

lavalamp · 2017-02-09T23:59:53Z

Sorry for delay in paying attention to this, I was in all day meetings last two days.

So, I have to admit I'm a little lost in the layers of automation here. I think I'd like us to do this:

Turn off the current publishing bot, since I think it's an auto-breakage bot right now.
Run this script, but instead of publishing directly, publish to Chao's fork, and make a PR.
The travis instance I turned on should test the PR, we can merge when it passes.
Hm, we should copy the travis.yml file to other branches, too.
Repeat the publishing process (steps 2-3) for every branch.

At that point, we should have a functioning client library, and I want to pause and regroup. Before we turn the automation back on, I want it to be able to test that the thing it's going to push actually works and will not break users.

I've started a client strategy doc here so we can all get on the same page. Anyone in the api machinery sig mailing list should have access: https://docs.google.com/document/d/1h_IBGYPMa8FS0oih4NbVkAMAzM7YTHr76VBcKy1qFbg/edit

caesarxuchao · 2017-02-10T13:49:37Z

Run this script, but instead of publishing directly, publish to Chao's fork, and make a PR.

If we want to do this, I'll need to submit a PR to apimachinery as well, it sounds complicated.

Alternatively, @deads2k @sttts if you can manually sync the apimachinery repo soon, I can manually fix client-go's master branch. How's that?

For client-go release-2.0 branch, the robot is doing the right thing. I'll wait for it to pick up my latest cherrypicks to the kubernetes release-1.5 branch, then disable the bot.

For release-1.4 and release-1.5, because they are not tracking any kubernetes branch, only manual fixes are possible.

I want it to be able to test that the thing it's going to push actually works and will not break users.

+100. How about letting the bot compile the client-go and run the unit tests before publishing?

caesarxuchao · 2017-02-13T10:21:42Z

I created the kubernetes-staging-publish-cops team and included @lavalamp @deads2k, @sttts and myself. This team will be notified if the robot fails to publish the staging folder to repos.

caesarxuchao · 2017-02-13T15:53:00Z

I'm verifying the code generated by the robot in kubernetes/client-go#103 and kubernetes/client-go#104. (The signal is if the travis test passes)

update: both travis tests have passed.

caesarxuchao · 2017-02-13T15:57:40Z

@lavalamp, the robot is generating sane code. I pushed the last commit which let the robot run go build and go testbefore publishing client-go. Please let me if there are other fundamental issues with this PR. I'll address the rest comments tomorrow.

caesarxuchao · 2017-02-14T07:24:05Z

Comments addressed. PTAL. Thanks.

lavalamp

I'm really sorry it took so long for me to find time to look at this.

I wonder if we can do this in three steps:

We get the shell scripts set up and runable by a human.
We get the publisher running them automatically
We add issue filing.

I wonder if we can make the scripts publish PRs instead of pushing directly. That would be much safer?

lavalamp · 2017-02-15T23:00:53Z

mungegithub/mungers/publisher.go

 	curDir, err := os.Getwd()
 	if err != nil {
-		glog.Infof("Getwd failed")
+		p.plog.Infof("Getwd failed")


You should actually print out the error in all of these, so people will know what to do.

lavalamp · 2017-02-15T23:04:14Z

mungegithub/mungers/publisher_logger.go

+limitations under the License.
+*/
+
+// Changing glog output directory via --log_dir doesn't work, because the flag


Can we just record to a bytes.Buffer or some such, print to the logs when we're done? I'm not sure we need this whole concept.

lavalamp · 2017-02-15T23:04:44Z

mungegithub/mungers/publisher_issue.go

+	return nil
+}
+
+func (p *publisherIssueTracker) FileIssue(failure publisherFailure) {


Can we put filing issues into a separate PR?

lavalamp · 2017-02-15T23:05:38Z

mungegithub/mungers/publish_scripts/apimachinery_sync_from_kubernetes.sh

+NETRCDIR="${2}"
+
+# set up github token
+echo "machine github.com login ${TOKEN}" > "${NETRCDIR}"/.netrc


why do you need a directory for this?

lavalamp · 2017-02-15T23:06:28Z

mungegithub/mungers/publish_scripts/apimachinery_sync_from_kubernetes.sh

+
+# set up github token
+echo "machine github.com login ${TOKEN}" > "${NETRCDIR}"/.netrc
+rm -f ~/.netrc


What if they need this?

lavalamp · 2017-02-15T23:20:00Z

mungegithub/mungers/publish_scripts/clientgo_publish.sh

-if [ ! $# -eq 5 ]; then
-    echo "usage: publish.sh destination_dir destination_branch token netrc_dir commit_message. destination_dir and netrc_dir are expected to be absolute paths."
+if [ ! $# -eq 6 ]; then
+    echo "usage: publish.sh destination_dir destination_branch token netrc_dir commit_message gopath. destination_dir and netrc_dir are expected to be absolute paths."


why take a gopath arg? Can we make a tmp dir instead?

And if not, why not expect $GOPATH to just be set correctly?

Doesn't matter now, no GOPATH needed.

lavalamp · 2017-02-15T23:22:50Z

mungegithub/mungers/publish_scripts/clientgo_publish.sh

@@ -43,6 +44,23 @@ if git diff --cached --exit-code &>/dev/null; then
    exit 0
 fi
 git commit -m "${MESSAGE}"
+
+# Run "godep restore" to restore dependencies. Because entries for


Prefix with "client-go's /vendor directory in staging doesn't include the other k8s.io/... dependencies, specifically apimachinery. Therefore, we do a restore/save cycle to fix this before publishing to the client repo.

Is there a reason why we have to do this here and can't just fix the vendor directory in the staging directory?

vendor/ in staging directory is fixed now, so i'm going to rewrite this part of code to simply replace the SHA of k8s.io/apimachinery in Godeps.json, but not update vendor/. Then the script doesn't need to go through the godep save/restore.

Fixing this now. Depends on kubernetes/kubernetes#42084.

lavalamp · 2017-02-15T23:26:03Z

mungegithub/publisher/deployment.yaml

@@ -23,6 +23,8 @@ spec:
        - --repo-dir=$(REPO_DIR)
        - --netrc-dir=$(NETRC_DIR)
        - --alsologtostderr
+        - --publisher-log-dir=$(PUBLISHER_LOG_DIR)


I recommend storing the log in memory instead of needing a separate file.

lavalamp · 2017-02-15T23:32:28Z

mungegithub/mungers/publisher.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2016 The Kubernetes Authors.
+Copyright 2017 The Kubernetes Authors.


Don't change years.

lavalamp · 2017-02-15T23:34:35Z

mungegithub/mungers/publisher.go

 // EachLoop is called at the start of every munge loop
 func (p *PublisherMunger) EachLoop() error {
+	// initialize the issueTracker in EachLoop, in case there is a new issue created in last EachLoop


I'm really super nervous about having this much untested code.

sttts · 2017-02-22T08:21:12Z

What is the status here? How can we help to finish this?

caesarxuchao · 2017-02-22T17:03:40Z

I'm working on addressing the comments.

caesarxuchao · 2017-02-27T21:14:04Z

Status update:
I'm refactoring client-go to use the filter-branch magic, this will save the commit history, and largely clean up the go code. After this is done, I'll open a PR with only the publish_client_go.sh and publish_apimachinery, and verify the scripts are good.

k8s.io/client-go. In the meantime, the publishing robot lets client-go vendor the just published k8s.io/apimachinery. add the ability to create an issue if publish fails don't delete .travis.yml go build && go test before publish client-go

…rsions of git

…there

k8s-ci-robot · 2017-02-28T23:24:26Z

@caesarxuchao: The following test(s) failed:

Test name	Commit	Details	Rerun command
Bazel test	`833de84`	link	`@k8s-bot bazel test this`

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.

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Feb 3, 2017

This was referenced Feb 3, 2017

Switch to dep for dependency management kubernetes/client-go#78

Closed

Remove apimachinery from staging client-go/Godeps/Godeps.json kubernetes/kubernetes#40909

Merged

k8s.io/client-go vendors k8s.io/apimachinery which breaks build kubernetes/client-go#83

Closed

deads2k reviewed Feb 3, 2017

View reviewed changes

sttts reviewed Feb 3, 2017

View reviewed changes

lavalamp reviewed Feb 3, 2017

View reviewed changes

lavalamp assigned mml Feb 3, 2017

sttts reviewed Feb 6, 2017

View reviewed changes

deads2k mentioned this pull request Feb 9, 2017

Example of a basic API server kubernetes/apiserver#2

Closed

caesarxuchao force-pushed the publish-apimachinery-and-client-go branch from f1789b2 to d325d58 Compare February 9, 2017 14:22

lavalamp reviewed Feb 9, 2017

View reviewed changes

sttts mentioned this pull request Feb 10, 2017

add k8s.io/sample-apiserver to demonstrate how to build an aggregated API server kubernetes/kubernetes#41136

Merged

caesarxuchao mentioned this pull request Feb 11, 2017

add back travis kubernetes/client-go#96

Merged

caesarxuchao force-pushed the publish-apimachinery-and-client-go branch 2 times, most recently from 7ec546b to dd4b80a Compare February 14, 2017 07:21

lavalamp suggested changes Feb 15, 2017

View reviewed changes

sttts mentioned this pull request Feb 22, 2017

Last sync is from Feb 3. kubernetes/client-go#123

Closed

fejta assigned lavalamp and sttts Feb 24, 2017

Chao Xu added 4 commits February 27, 2017 13:51

remove workaround for different commit SHAs generated by different ve…

03684ad

…rsions of git

update bazel

c0a4bc6

addressing comments

1bdaf2d

sttts mentioned this pull request Feb 28, 2017

Create k8s.io/apimachinery repo kubernetes/kubernetes#39528

Closed

8 tasks

convert publish_client_go.sh to use the 'git filter-branch', halfway …

833de84

…there

caesarxuchao force-pushed the publish-apimachinery-and-client-go branch from d2b06d3 to 833de84 Compare February 28, 2017 23:23

caesarxuchao mentioned this pull request Feb 28, 2017

Add scripts that publish repos #2077

Merged

caesarxuchao closed this Mar 16, 2017

		@@ -46,6 +49,10 @@ func (c coordinate) String() string {
		type repoRules struct {

Let the publishing robot publish k8s.io/apimachinery and k8s.io/client-go #1784

Let the publishing robot publish k8s.io/apimachinery and k8s.io/client-go #1784

Conversation

caesarxuchao commented Feb 3, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deads2k commented Feb 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deads2k commented Feb 3, 2017

sttts commented Feb 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

caesarxuchao commented Feb 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sttts commented Feb 6, 2017

caesarxuchao commented Feb 7, 2017

sttts commented Feb 7, 2017

caesarxuchao commented Feb 7, 2017

k8s-reviewable commented Feb 7, 2017

foxish commented Feb 8, 2017

caesarxuchao commented Feb 8, 2017

caesarxuchao commented Feb 8, 2017

caesarxuchao commented Feb 9, 2017

sttts commented Feb 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lavalamp commented Feb 9, 2017

caesarxuchao commented Feb 10, 2017

caesarxuchao commented Feb 13, 2017 • edited Loading

caesarxuchao commented Feb 13, 2017 • edited Loading

caesarxuchao commented Feb 13, 2017

caesarxuchao commented Feb 14, 2017

lavalamp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

caesarxuchao Feb 24, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sttts commented Feb 22, 2017

caesarxuchao commented Feb 22, 2017

caesarxuchao commented Feb 27, 2017

k8s-ci-robot commented Feb 28, 2017

caesarxuchao commented Feb 3, 2017 •

edited

Loading

caesarxuchao commented Feb 13, 2017 •

edited

Loading

caesarxuchao commented Feb 13, 2017 •

edited

Loading

caesarxuchao Feb 24, 2017 •

edited

Loading