kubecfg 0.8.0 (new formula) #25737

aecolley · 2018-03-25T13:37:20Z

This is a Kubernetes tool developed at Bitnami. It is a prerequisite
for some development tasks with the better-known Kubeless project
(http://kubeless.io/docs/dev-guide/). It should not be confused with
the identically-named, defunct precursor to kubectl.

I am slightly concerned that it doesn't have its own website, except
for its Github repository and a couple of YouTube videos. But a quick
survey shows that 25% of homebrew-core formulae have homepages in
the github.com domain, so I suspect it's OK.

[✓] Have you followed the guidelines for contributing?
[✓] Have you checked that there aren't other open pull requests for the same formula update/change?
[✓] Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
[✓] Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

This is a Kubernetes tool developed at Bitnami. It is a prerequisite for some development tasks with the better-known Kubeless project (http://kubeless.io/docs/dev-guide/). It should not be confused with the identically-named, defunct precursor to kubectl. I am slightly concerned that it doesn't have its own website, except for its Github repository and a couple of YouTube videos. But a quick survey shows that 25% of homebrew-core formulae have homepages in the github.com domain, so I suspect it's OK. The "gopath shenanigans" mentioned in a comment seem to be a new requirement with Go 1.10. Previous Go releases let you get away with a symlink to buildpath, but 1.10 refuses to recognise the "vendor" directory if it's found via a symlink.

jjo · 2018-03-26T14:49:46Z

Thanks @aecolley for the PR, much appreciated !, will loop
someone else with brew'ing skills already to review it :)

andresmgot

I tested the formula and the installation, audit and test worked as expected. I just made some small comments. Apart from that wait for brew maintainers to have more details about other requirements they may have. Thanks!

andresmgot · 2018-03-27T07:50:13Z

Formula/kubecfg.rb

+class Kubecfg < Formula
+  desc "Manage complex enterprise Kubernetes environments as code"
+  homepage "https://github.com/ksonnet/kubecfg"
+  url "https://github.com/ksonnet/kubecfg/archive/v0.7.2.tar.gz"


for making it easier to update you can use the github repo as url and the tag and commit as versions:

url "https://github.com/ksonnet/kubecfg.git" :tag => "v0.7.2", :revision => "7c1e6e59cc5cdf7259f17d349e55b6287b419b13"

While that works fine, the Acceptable Formulae document says:

Tarballs are preferred to Git checkouts, and tarballs should include the version in the filename whenever possible.

Unless you specifically overrule that, I'm inclined to hew closely to it.

andresmgot · 2018-03-27T07:50:44Z

Formula/kubecfg.rb

+      args.push("VERSION=#{version}") unless build.head?
+      system "make", "generate", *args if build.head?
+      system "make", *args
+      system "make", "test", *args if build.head?


I would move the make test task if possible to the test hook

That's a good idea, but I'm not sure it's possible. The Formula API says that buildpath is only non-nil inside the install method. I don't see another way to find the extracted source code, or even a way to provoke its recreation. The man page describes brew test as something that

should generally indicate to the user if something is wrong with the installed formula.

andresmgot · 2018-03-27T07:55:59Z

Formula/kubecfg.rb

+  end
+
+  test do
+    system "#{bin}/kubecfg", "show", "#{share}/lib/kubecfg_test.jsonnet"


I would compare the output of this command with an expected result

There's no need to compare the output. The test file uses asserts everywhere and is designed to cause a non-zero exit on failure. The "success" value
is just the bare minimum designed to cause kubecfg to exit successfully.

No problem.

I reverted this after reading @anguslees' reply.

anguslees · 2018-03-28T00:02:21Z

Formula/kubecfg.rb

+    sharelib = share/"lib"
+    sharelib.mkpath
+    sharelib.install srcdir/"lib/kubecfg.libsonnet"
+    sharelib.install srcdir/"lib/kubecfg_test.jsonnet"


This _test lib is not relevant for an end user, and doesn't need to be installed.

That's true. I installed it only so it would be available for the test mode. If you come up with a good one-liner (for values of one up to 5) which tests the most basic functionality of kubecfg, I'll happily substitute it.

Oh I see - the homebrew "test" is on the installed result, not the intermediate build artifacts. In that case, it's a reasonable test 👍

anguslees · 2018-03-28T00:04:10Z

Formula/kubecfg.rb

+    bin.install srcdir/"kubecfg"
+    sharelib = share/"lib"
+    sharelib.mkpath
+    sharelib.install srcdir/"lib/kubecfg.libsonnet"


Note that the next and future releases (from vmware-archive/kubecfg#197) won't need this file to be separately installed - it now embeds kubecfg.libsonnet in the binary.

Current (and past) releases do need this separately installed, however.

Edit: Ah! I see from the caveats text below, that you're already aware of this, and just installing it as a reference for the user. Nice move.

Ah, I thought it was already embedded in 0.7.2.

I included a tiny patch to inject the share/lib directory at the end of the search path. import "kubecfg.jsonnet" now looks in the right place. Obviously it should go on the next release with the internalfs stuff obviating it.

Is this still needed for 0.8.0?

No, but it's used in the brew test block.

anguslees · 2018-03-28T00:06:01Z

Formula/kubecfg.rb

+    cd srcdir do
+      args = []
+      args.push("VERSION=#{version}") unless build.head?
+      system "make", "generate", *args if build.head?


make generate (and go-bindata) are only required at dev-time, and the up-to-date generated file is supposed to be already included in git repo. .. So I think you should remove this step (and go-bindata dependency) from the homebrew formula.

Simpler is better, so I'll do that.

anguslees · 2018-03-28T00:13:31Z

Formula/kubecfg.rb

+  end
+
+  test do
+    system "#{bin}/kubecfg", "show", "#{share}/lib/kubecfg_test.jsonnet"


There's no need to compare the output. The test file uses asserts everywhere and is designed to cause a non-zero exit on failure. The "success" value
is just the bare minimum designed to cause kubecfg to exit successfully.

This reverts commit 55ca312.

anguslees · 2018-03-28T01:17:04Z

Formula/kubecfg.rb

+    # The real build steps:
+    cd srcdir do
+      args = []
+      args.push("VERSION=#{version}") unless build.head?


what does #{version} look like once expanded? It would be good if it matched the regular kubecfg upstream release builds, which use git tag - eg v0.7.2 (including the v) and not just 0.7.2.

Added the v.

anguslees

lgtm.

(and hi aecolley! :)

…associated lies from caveats

anguslees

(still lgtm)

anguslees · 2018-03-29T02:38:06Z

"At least 6 approving reviews are required by reviewers with write access." Wow.

aecolley · 2018-04-03T06:24:31Z

@fxcoudert is this ready for review, or is there something else I should do first?

(Nice work on #26034 by the way!)

aecolley · 2018-04-17T09:04:49Z

@fxcoudert two-week ping

commitay · 2018-04-17T20:35:06Z

@BrewTestBot test this please

commitay · 2018-04-17T20:51:55Z

Formula/kubecfg.rb

+  homepage "https://github.com/ksonnet/kubecfg"
+  url "https://github.com/ksonnet/kubecfg/archive/v0.8.0.tar.gz"
+  sha256 "25d054af96a817bad0f33998895a9988c187e1399822c8220528e64f56ccb3ae"
+  head "https://github.com/ksonnet/kubecfg.git"


Remove head

Really? I will comply, but I suggest that the Acceptable Formulae document should document the deprecation of head and any conditions that apply.

commitay · 2018-04-17T20:52:38Z

Formula/kubecfg.rb

+    bin.install srcdir/"kubecfg"
+    sharelib = share/"lib"
+    sharelib.mkpath
+    sharelib.install srcdir/"lib/kubecfg.libsonnet"


Is this still needed for 0.8.0?

commitay · 2018-04-17T20:55:59Z

Formula/kubecfg.rb

+  def install
+    ENV["GOPATH"] = buildpath
+    srcdir = buildpath/"src/github.com/ksonnet/kubecfg"
+    srcdir.install buildpath.children - [buildpath/".brew_home"]


ENV["GOPATH"] = buildpath (buildpath/"src/github.com/ksonnet/kubecfg").install buildpath.children cd "src/github.com/ksonnet/kubecfg" do system "make", "VERSION=v#{version}" bin.install "kubecfg" ...

Thanks! Added.

commitay · 2018-04-19T04:08:20Z

@BrewTestBot test this please

aecolley · 2018-04-27T12:49:17Z

Is there something further I should do here?

fxcoudert · 2018-04-30T13:21:42Z

@BrewTestBot test this please

.

fxcoudert · 2018-04-30T13:22:49Z

Formula/kubecfg.rb

+  end
+
+  test do
+    system bin/"kubecfg", "show", pkgshare/"kubecfg_test.jsonnet"


Can we match the output?

output = shell_output("#{bin}/kubecfg show #{pkgshare}/kubecfg_test.jsonnet") assert_match "result: SUCCESS", output

The command spawned by shell_output uses its exit status to indicate success or failure. The kubecfg_test.jsonnet uses assertions to carry out all of its testing, and the output is trivial. We already discussed this earlier in this PR.

ilovezfs · 2018-04-30T13:23:40Z

Formula/kubecfg.rb

+    (zsh_completion/"_kubecfg").write output
+  end
+
+  def caveats; <<~EOS


This is a standard location for Homebrew so I don't think we need this caveat

Edit: never mind, I see now the deletion that made my comment obsolete.

Without the caveat, the user would naturally assume that the installed file is used by the kubecfg binary, and therefore that editing the file would be a way to debug it. The truth is that editing the file would be a waste of time. It is installed as a form of documentation, nothing more. Also, this is the only mention of the KUBECFG_JPATH feature, other than the source code. I freely admit I'm abusing caveats as documentation.

stale · 2018-05-24T02:06:21Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

fxcoudert · 2018-05-24T08:18:50Z

@BrewTestBot test this please

ilovezfs · 2018-05-24T08:55:46Z

Thanks for your first contribution to Homebrew, and your patience, @aecolley! You're awesome.

aecolley mentioned this pull request Mar 25, 2018

Add homebrew install instructions? vmware-archive/kubecfg#68

Closed

fxcoudert added the new formula PR adds a new formula to Homebrew/homebrew-core label Mar 25, 2018

andresmgot reviewed Mar 27, 2018

View reviewed changes

anguslees reviewed Mar 28, 2018

View reviewed changes

aecolley added 3 commits March 28, 2018 01:53

Check output in test

55ca312

Revert "Check output in test"

8addd42

This reverts commit 55ca312.

Remove "make generate"

6c66568

anguslees reviewed Mar 28, 2018

View reviewed changes

Include v in VERSION

c803542

anguslees approved these changes Mar 28, 2018

View reviewed changes

Patch so that import "kubecfg.libsonnet" works on v0.7.2, and remove …

feeeca3

…associated lies from caveats

anguslees approved these changes Mar 29, 2018

View reviewed changes

Update to v0.8.0 release, and import some boilerplate from kubectl.rb

cd04422

aecolley changed the title ~~kubecfg 0.7.2 (new formula)~~ kubecfg 0.8.0 (new formula) Apr 3, 2018

commitay previously requested changes Apr 17, 2018

View reviewed changes

aecolley and others added 3 commits April 18, 2018 12:28

decapitate

8a4e292

simplify install steps

bfaf248

Update kubecfg.rb

b28b1ae

fxcoudert requested changes Apr 30, 2018

View reviewed changes

ilovezfs reviewed Apr 30, 2018

View reviewed changes

stale bot added the stale No recent activity label May 24, 2018

stale bot removed the stale No recent activity label May 24, 2018

fxcoudert approved these changes May 24, 2018

View reviewed changes

Update kubecfg.rb

57e70f1

fxcoudert added the ready to merge PR can be merged once CI is green label May 24, 2018

ilovezfs closed this in ccbae0f May 24, 2018

lock bot added the outdated PR was locked due to age label Jun 23, 2018

lock bot locked as resolved and limited conversation to collaborators Jun 23, 2018

kubecfg 0.8.0 (new formula) #25737

kubecfg 0.8.0 (new formula) #25737

Conversation

aecolley commented Mar 25, 2018 • edited

jjo commented Mar 26, 2018

andresmgot 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

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

anguslees Mar 28, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anguslees left a comment

Choose a reason for hiding this comment

anguslees left a comment

Choose a reason for hiding this comment

anguslees commented Mar 29, 2018

aecolley commented Apr 3, 2018

aecolley commented Apr 17, 2018

commitay commented Apr 17, 2018

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

commitay commented Apr 19, 2018

aecolley commented Apr 27, 2018

fxcoudert commented Apr 30, 2018

fxcoudert Apr 30, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aecolley May 3, 2018 • edited

Choose a reason for hiding this comment

stale bot commented May 24, 2018

fxcoudert commented May 24, 2018

ilovezfs commented May 24, 2018

aecolley commented Mar 25, 2018 •

edited

anguslees Mar 28, 2018 •

edited

fxcoudert Apr 30, 2018 •

edited

aecolley May 3, 2018 •

edited