Repository: puppetlabs/wash
Pull request #508 (2019-09-24 20:33:22)

Omit extra newline when printing winfo and meta output
Signed-off-by: Enis Inan <enis.inan@puppet.com>


Patch:
diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml
index 5e21a2e..badf14a 100644
--- a/.github/workflows/push.yml
+++ b/.github/workflows/push.yml
@@ -20,7 +20,7 @@ jobs:
         git rm -r '*'
         touch .nojekyll
       env:
-        GITHUB_TOKEN: ${{ secrets.HOMEBREW_DEPLOY_TOKEN }}
+        GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
     - name: Checkout latest tag
       run: git checkout $(git describe --tags $(git rev-list --tags --max-count=1))
     - name: Build release website
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 27ada64..08a3ab0 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -10,11 +10,11 @@ Browse for existing issues. Otherwise, open a "[new issue](https://github.com/pu
 
 ## How to report a bug
 
-Open a "[Bug report](https://github.com/puppetlabs/wash/issues/new?template=bug-report.md)" issue in this repo.
+Open a "[Bug report](https://github.com/puppetlabs/wash/issues/new?template=bug_report.md)" issue in this repo.
 
 ## How to suggest a new feature
 
-Open a "[Feature request](https://github.com/puppetlabs/wash/issues/new?template=feature-request.md)" issue in this repo.
+Open a "[Feature request](https://github.com/puppetlabs/wash/issues/new?template=feature_request.md)" issue in this repo.
 
 ## Development Environment
 
@@ -41,17 +41,15 @@ We try to follow common Go conventions as enforced by the compiler and several s
 
 File naming should follow camelCase. When grouping several files around a single concept, multiple names can be joined by hyphens.
 
-For example, the Docker plugin has multiple files implementing components of a container. We name them
+For example, the Docker plugin has multiple files implenting components of a container. We name them
 - container-log.go
 - container-metadata.go
 - container.go
 
 ### Releases
 
-Releases are done through the GitHub's [Draft a new release](https://github.com/puppetlabs/wash/releases/new). Assets are automatically built and a [GitHub Action](https://github.com/puppetlabs/wash/blob/master/.github/workflows/release.yml) will open a PR against https://github.com/puppetlabs/homebrew-puppet to update to the new release.
+Releases are done through the GitHub's [Draft a new release](https://github.com/puppetlabs/wash/releases/new). Assets are automatically built. https://github.com/puppetlabs/homebrew-puppet needs to be manually updated to use the new release (in the future we hope to automate this with GitHub Actions).
 
 Release announcements are:
-
-- posted in [Slack #cloudnative](https://puppetcommunity.slack.com/app_redirect?channel=cloudnative) (the release is automatically posted to #wash by the GitHub App); ask a moderator to re-post into #announcements. Example announcement:
-  > Wash x.y.z is now available. This release added ... Detailed release notes are available at https://github.com/puppetlabs/wash/releases/tag/x.y.z.
-- Puppet internal [release announcement process](https://confluence.puppetlabs.com/display/PM/Sending+Product+Release+Announcements) sends to puppet-announce@ and puppet-users@
+- posted in [Slack #puppet](https://puppetcommunity.slack.com/app_redirect?channel=puppet); ask a moderator to re-post into #announcements
+- Puppet internal [release announcement process](https://confluence.puppetlabs.com/display/PM/Sending+Product+Release+Announcements)
diff --git a/activity/core.go b/activity/core.go
index 1fef37e..736ac1b 100644
--- a/activity/core.go
+++ b/activity/core.go
@@ -128,21 +128,38 @@ func SubmitMethodInvocation(ctx context.Context, plugin string, entryType string
 		journal.addToHistory()
 	}
 
-	journal.recorder().submitMethodInvocation(entryType, method, func() {
-		log.Debugf("Submitting %v method invocation for entry instance %v", method, entryType)
-		err := analytics.GetClient(ctx).Event(
-			"Invocation",
-			"Method",
-			analytics.Params{
-				"Label":  method,
-				"Plugin": plugin,
-			},
-		)
-		if err != nil {
-			// We should never hit this code-path
-			panic(fmt.Sprintf("Unexpected error when submitting the method invocation: %v", err))
-		}
-	})
+	recorder := journal.recorder()
+
+	// Check if the method was already invoked
+	recorder.mIMux.RLock()
+	if recorder.methodInvoked(entryType, method) {
+		recorder.mIMux.RUnlock()
+		return
+	}
+	// Method wasn't invoked
+	recorder.mIMux.RUnlock()
+
+	recorder.mIMux.Lock()
+	defer recorder.mIMux.Unlock()
+
+	// Check the invocations again in case someone beat us to it
+	if recorder.methodInvoked(entryType, method) {
+		return
+	}
+	log.Debugf("Submitting %v method invocation for entry instance %v", method, entryType)
+	err := analytics.GetClient(ctx).Event(
+		"Invocation",
+		"Method",
+		analytics.Params{
+			"Label":  method,
+			"Plugin": plugin,
+		},
+	)
+	if err != nil {
+		// We should never hit this code-path
+		panic(fmt.Sprintf("Unexpected error when submitting the method invocation: %v", err))
+	}
+	recorder.recordMethodInvocation(entryType, method)
 }
 
 func closeRecorder(id string, obj interface{}) {
diff --git a/activity/journal.go b/activity/journal.go
index 6d382bb..f9821b6 100644
--- a/activity/journal.go
+++ b/activity/journal.go
@@ -198,42 +198,24 @@ func newRecorder() recorder {
 }
 
 // methodInvoked returns true if "method" was invoked by an instance
-// of "entryType". methodInvoked is not thread-safe and should be called
-// with `r.mIMux.RLock()`.
+// of "entryType". methodInvoked is not thread-safe.
 func (r recorder) methodInvoked(entryType string, method string) bool {
-	methodInvocations, ok := r.methodInvocations[entryType]
-	if !ok {
-		return false
-	}
+	methodInvocations := r.methodInvocationsOf(entryType)
 	return methodInvocations[method]
 }
 
-// submitMethodInvocation calls submitter and records an invocation of
-// "method" by an instance of "entryType" only if the "method" has not
-// previously been invoked by an instance of "entryType".
-func (r recorder) submitMethodInvocation(entryType string, method string, submitter func()) {
-	r.mIMux.RLock()
-	if r.methodInvoked(entryType, method) {
-		r.mIMux.RUnlock()
-		return
-	}
-	r.mIMux.RUnlock()
+// recordMethodInvocation records an invocation of "method" by an instance
+// of "entryType". recordMethodInvocation is not thread-safe.
+func (r recorder) recordMethodInvocation(entryType string, method string) {
+	methodInvocations := r.methodInvocationsOf(entryType)
+	methodInvocations[method] = true
+}
 
-	r.mIMux.Lock()
+func (r recorder) methodInvocationsOf(entryType string) methodInvocations {
 	methodInvocations, ok := r.methodInvocations[entryType]
 	if !ok {
 		methodInvocations = make(map[string]bool)
 		r.methodInvocations[entryType] = methodInvocations
 	}
-	if methodInvocations[method] {
-		r.mIMux.Unlock()
-		return
-	}
-
-	methodInvocations[method] = true
-	r.mIMux.Unlock()
-
-	// We don't need to lock while submitting. If the submission fails we move on instead of retrying
-	// so we can mark that we tried before even submitting.
-	submitter()
+	return methodInvocations
 }
diff --git a/cmd/info.go b/cmd/info.go
index 3f548cf..9fdb942 100644
--- a/cmd/info.go
+++ b/cmd/info.go
@@ -61,7 +61,7 @@ func infoMain(cmd *cobra.Command, args []string) exitCode {
 		return exitCode{1}
 	}
 
-	cmdutil.Println(marshalledEntry)
+	cmdutil.Print(marshalledEntry)
 
 	return exitCode{0}
 }
diff --git a/cmd/meta.go b/cmd/meta.go
index ebfdaa0..de33748 100644
--- a/cmd/meta.go
+++ b/cmd/meta.go
@@ -61,7 +61,7 @@ func metaMain(cmd *cobra.Command, args []string) exitCode {
 		return exitCode{1}
 	}
 
-	cmdutil.Println(prettyMetadata)
+	cmdutil.Print(prettyMetadata)
 
 	return exitCode{0}
 }
diff --git a/plugin/gcp/computeInst.go b/plugin/gcp/computeInst.go
index 8d470a2..f966965 100644
--- a/plugin/gcp/computeInst.go
+++ b/plugin/gcp/computeInst.go
@@ -12,7 +12,6 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
-	"time"
 
 	"github.com/puppetlabs/wash/activity"
 	"github.com/puppetlabs/wash/plugin"
@@ -34,15 +33,9 @@ func newComputeInstance(inst *compute.Instance, c computeProjectService) *comput
 		instance:  inst,
 		service:   c,
 	}
-	crtime, err := time.Parse(time.RFC3339, inst.CreationTimestamp)
-	if err != nil {
-		panic(fmt.Sprintf("Timestamp for %v was not expected format RFC3339: %v", comp, inst.CreationTimestamp))
-	}
 	comp.
 		DisableCachingFor(plugin.MetadataOp).
-		Attributes().
-		SetCrtime(crtime).
-		SetMeta(inst)
+		Attributes().SetMeta(inst)
 	return comp
 }
 
diff --git a/plugin/gcp/storageBucket.go b/plugin/gcp/storageBucket.go
index 1f6856b..8b2749c 100644
--- a/plugin/gcp/storageBucket.go
+++ b/plugin/gcp/storageBucket.go
@@ -17,11 +17,7 @@ type storageBucket struct {
 
 func newStorageBucket(client storageProjectClient, bucket *storage.BucketAttrs) *storageBucket {
 	stor := &storageBucket{EntryBase: plugin.NewEntry(bucket.Name), storageProjectClient: client}
-	stor.Attributes().
-		SetCrtime(bucket.Created).
-		SetCtime(bucket.Created).
-		SetMtime(bucket.Created).
-		SetMeta(bucket)
+	stor.Attributes().SetMeta(bucket)
 	return stor
 }
 
diff --git a/plugin/gcp/storageObject.go b/plugin/gcp/storageObject.go
index f97ae05..ac148df 100644
--- a/plugin/gcp/storageObject.go
+++ b/plugin/gcp/storageObject.go
@@ -15,12 +15,7 @@ type storageObject struct {
 
 func newStorageObject(name string, object *storage.ObjectHandle, attrs *storage.ObjectAttrs) *storageObject {
 	obj := &storageObject{EntryBase: plugin.NewEntry(name), ObjectHandle: object}
-	obj.Attributes().
-		SetCrtime(attrs.Created).
-		SetCtime(attrs.Updated).
-		SetMtime(attrs.Updated).
-		SetSize(uint64(attrs.Size)).
-		SetMeta(attrs)
+	obj.Attributes().SetMtime(attrs.Updated).SetSize(uint64(attrs.Size)).SetMeta(attrs)
 	return obj
 }
 
diff --git a/plugin/gcp/storageObjectPrefix.go b/plugin/gcp/storageObjectPrefix.go
index 9d0a1ec..91b7543 100644
--- a/plugin/gcp/storageObjectPrefix.go
+++ b/plugin/gcp/storageObjectPrefix.go
@@ -23,12 +23,7 @@ func newStorageObjectPrefix(bucket *storage.BucketHandle,
 		prefix:    prefix,
 	}
 	if attrs != nil {
-		pre.Attributes().
-			SetCrtime(attrs.Created).
-			SetCtime(attrs.Updated).
-			SetMtime(attrs.Updated).
-			SetSize(uint64(attrs.Size)).
-			SetMeta(attrs)
+		pre.Attributes().SetMtime(attrs.Updated).SetSize(uint64(attrs.Size)).SetMeta(attrs)
 	}
 	return pre
 }
diff --git a/website/content/_index.md b/website/content/_index.md
index a0c362b..ef09552 100644
--- a/website/content/_index.md
+++ b/website/content/_index.md
@@ -131,7 +131,7 @@ We've implemented a number of handy Wash commands ([docs](docs#wash-commands)):
 
 * `wash ls` - a version of `ls` that uses our API to enhance directory listings with Wash-specific info
   - _e.g. show you what primitives are supported for each resource_
-* `wash meta` - emits a resource's metadata to standard output
+* `wash meta` - emits a resource's metadata to standard out
 * `wash exec` - uses the `exec` primitive to let you invoke commands against resources
 * `wash find` - find resources using powerful selection predicates
 * `wash tail -f` - follow updates to resources that support the `stream` primitive as well as normal files
diff --git a/website/content/posts/introducing-wash.md b/website/content/posts/introducing-wash.md
index 87db019..7a9f1d6 100644
--- a/website/content/posts/introducing-wash.md
+++ b/website/content/posts/introducing-wash.md
@@ -13,35 +13,37 @@ Have you ever had to:
 
 If so, then some parts of the following tables might look familiar to you. If not, then here's how AWS/Docker/Kubernetes/GCP recommends that you do some of these tasks.
 
-List all... | Command
+<div style="width:200px">List all</div> | 
 ----------------------|---
 AWS EC2 instances     | `aws ec2 describe-instances --profile foo --query 'Reservations[].Instances[].InstanceId' --output text`
 Docker containers     | `docker ps --all`
 Kubernetes pods       | `kubectl get pods --all-namespaces`
 GCP Compute instances | `gcloud compute instances list`
 
-Read... | Command
+<div style="width:200px">Read</div>         | 
 --------------------------------------------|---
 Console output of an EC2 instance           | `aws ec2 get-console-output --profile foo --instance-id bar`
 Console output of a Google compute instance | `gcloud compute instances get-serial-port-output foo`
 An S3 object's content                      | `aws s3api get-object content.txt --profile foo --bucket bar --key baz && cat content.txt && rm content.txt`
 A GCP Storage object's content              | `gsutil cat gs://foo/bar`
 
-Exec `uname` on... | Command
+<div style="width:200px">Exec `uname` on</div> | 
 -----------------------------|---
 An EC2 instance              | `ssh -i /path/my-key-pair.pem ec2-user@195.70.57.35 uname`
 An a Docker container        | `docker exec foo uname`
 Exec on a Kubernetes pod     | `kubectl exec foo uname`
 On a Google Compute instance | `gcloud compute ssh foo --command uname`
 
-Find by 'owner'... | Command
+<div style="width:200px">Find by 'owner' tag/label</div> | 
 --------------------------|---
 EC2 instances             | `aws ec2 describe-instances --profile foo --query 'Reservations[].Instances[].InstanceId' --filters Name=tag-key,Values=owner --output text`
 Docker containers         | `docker ps --filter “label=owner”`
 Kubernetes pods           | `kubectl get pods --all-namespaces --selector=owner`
 Google Compute instance   | `gcloud compute instances list --filter=”labels.owner:*”`
 
-From this, we see that you need to use different commands to List/Read/Exec/Find different things. Furthermore, these commands require you to install different applications that each come with their own set of (possibly conflicting) dependencies, and their own calling conventions. For example, to complete all the Find tasks specified in the table, you need to:
+<p style="text-align: center;">Table 1: Common CLI Tasks</p>
+
+From Table 1, we see that you need to use different commands to List/Read/Exec/Find different things. Furthermore, these commands require you to install different applications that each come with their own set of (possibly conflicting) dependencies, and their own calling conventions. For example, to complete all the Find tasks specified in the table, you need to:
 
 * Use the `aws ec2 describe-instances`, `docker ps`, `kubectl get pods`, `gcloud compute instances list` commands (4 different commands).
 
@@ -49,41 +51,39 @@ From this, we see that you need to use different commands to List/Read/Exec/Find
 
 * Learn four different-but-similar DSLs for filtering stuff, which effectively means four different-but-similar ways of constructing and combining predicates on structured data (e.g. GCP's filter expressions, Kubernetes' field selectors, Kubernetes' label selectors, aws' describe-instances' --filters option, docker ps filtering, etc.).
 
-That's a lot of stuff you have to use and learn to do some very fundamental and basic tasks. It naturally begs the question of whether there's a better way of performing these tasks that is (1) more expressive than what's presented here, and (2) does not require you to learn different commands to perform a task on different things.
-
-The answer is Wash. With Wash, here's what the invocations look like:
+That's a lot of stuff you have to use and learn to do some very fundamental and basic tasks. It naturally begs the question of whether there's a better way of performing these tasks that is (1) more expressive than what's presented here, and (2) does not require you to learn different commands to perform a task on different things. The answer is Wash. With Wash, here's what the invocations look like:
 
-List all... | Command
+<div style="width:200px">List all</div> | 
 ----------------------|---
 AWS EC2 instances     | `find aws/foo -k '*ec2*instance'`
 Docker containers     | `find docker -k '*container' `
 Kubernetes pods       | `find kubernetes -k '*pod'`
 GCP Compute instances | `find gcp -k '*compute*instance'`
 
-Read... | Command
+<div style="width:200px">Read</div>         | 
 --------------------------------------------|---
 Console output of an EC2 instance           | `cat aws/foo/resources/ec2/instances/bar/console.out`
 Console output of a Google compute instance | `cat gcp/<project>/compute/foo/console.out`
 An S3 object's content                      | `cat aws/foo/resources/s3/bar/baz`
 A GCP Storage object's content              | `cat gcp/<project>/storage/foo/bar`
 
-Exec `uname` on... | Command
+<div style="width:200px">Exec `uname` on </div> | 
 -----------------------------|---
 An EC2 instance              | `wexec aws/foo/resources/ec2/instances/bar uname`
 An a Docker container        | `wexec docker/containers/foo uname`
 Exec on a Kubernetes pod     | `wexec kubernetes/<context>/<namespace>/pods/foo uname`
 On a Google Compute instance | `wexec gcp/<project>/compute/foo uname`
 
-Find by 'owner'... | Command
+<div style="width:200px">Find by 'owner' tag/label</div> | 
 ------------------------|---
 EC2 instances           | `find aws/foo -k '*ec2*instance' -meta '.tags[?].key' owner`
 Docker containers       | `find docker -k '*container' -meta '.labels.owner' -exists`
 Kubernetes pods         | `find kubernetes -k '*pod' -meta '.metadata.labels.owner' -exists`
 Google Compute instance | `find gcp -k '*compute*instance' -meta '.labels.owner' -exists`
 
-Contrast this with the commands you'd ordinarily have to use. We immediately see that using Wash means:
+<p style="text-align: center;">Table 2: Common CLI Tasks with Wash</p>
 
-* The commands you use follow established, UNIX conventions.
+Comparing Tables 1 and 2, we immediately see that using Wash means:
 
 * You no longer have to learn different commands to execute a task across different things. All you need is one command (`find` for List/Find; `cat` for Read; and `wexec` for Exec).
 
@@ -99,6 +99,4 @@ In fact, Wash is not just a centralization of basic and fundamental commands. It
 
 Furthermore, Wash is built on a plugin architecture. It ships with some default plugins for Docker, AWS, Kubernetes, and GCP, but you can easily extend it with your own plugin via the external plugin interface (and you can write that plugin in any language you want, including Bash). People have written external plugins for all sorts of things such as IoT devices, Goodreads, GitHub, PuppetDB, and even Spotify.
 
-If this all sounds intriguing to you, please [download Wash](/#getting-started) Wash and [give it a try](/#wash-by-example)!
-
 [^1]: Notice that this example showcases another Wash feature: that you don't have to SSH into a bunch of different things to individually tail their logs. For example, something like `tail aws/foo/resources/ec2/instances/bar/fs/var/log/messages gcp/baz/compute/qux/fs/var/log/messages` will tail the `/var/log/messages` file on the bar EC2 instance and the qux GCP compute instance. As a fun example, the command `tail -f $(find -k 'aws/*ec2*instance' -o -k 'gcp/*compute*instance' | sed s:$:/fs/var/log/messages:g)` will tail the `/var/log/messages` file of every EC2 instance and GCP compute instance. (Note that the intermediate sed appends `/fs/var/log/messages` to the outputted paths. Baking this logic into find considerably slows things down. Also, `fs` is short for “filesystem”. It represents the root directory of the EC2 instance/GCP compute instance).
diff --git a/website/static/custom.css b/website/static/custom.css
index 3532eac..da68e26 100644
--- a/website/static/custom.css
+++ b/website/static/custom.css
@@ -86,22 +86,3 @@ blockquote {
     border-left-color: #ccc;
     padding-left: 10px;
 }
-
-table {
-    border-collapse: collapse;
-    border-spacing: 0;
-    empty-cells: show;
-    margin: 1em 0em;
-}
-
-thead {
-    background-color: #e0e0e0;
-    color: #000;
-    text-align: left;
-    vertical-align: bottom;
-}
-
-td, th {
-    border: 1px solid #cbcbcb;
-    padding: 0.5em 1em;
-}


Test patch:
diff --git a/activity/core_test.go b/activity/core_test.go
index a149262..3e6b812 100644
--- a/activity/core_test.go
+++ b/activity/core_test.go
@@ -107,7 +107,7 @@ func TestSubmitMethodInvocation_SubmittedMethodInvocation_DoesNotSubmitToGA(t *t
 		// Use a different journal ID so that we get a different recorder
 		ID: "bar",
 	}
-	journal.recorder().submitMethodInvocation("foo::file", "List", func() {})
+	journal.recorder().recordMethodInvocation("foo::file", "List")
 	ctx = context.WithValue(ctx, JournalKey, journal)
 	analyticsClient := &mockAnalyticsClient{}
 	analyticsClient.On("Event", mock.Anything, mock.Anything, mock.Anything).Return(nil)
diff --git a/activity/journal_test.go b/activity/journal_test.go
index 5bea71c..fd7c427 100644
--- a/activity/journal_test.go
+++ b/activity/journal_test.go
@@ -4,7 +4,6 @@ import (
 	"context"
 	"io/ioutil"
 	"regexp"
-	"sync"
 	"testing"
 	"time"
 
@@ -64,41 +63,13 @@ func TestHistoryWithJournal(t *testing.T) {
 
 func TestRecorder_CanRecordMethodInvocations(t *testing.T) {
 	recorder := newRecorder()
-	var invoked bool
-	invoker := func() {
-		invoked = true
-	}
 
 	assert.False(t, recorder.methodInvoked("foo", "List"))
-	recorder.submitMethodInvocation("foo", "List", invoker)
+	recorder.recordMethodInvocation("foo", "List")
 	assert.True(t, recorder.methodInvoked("foo", "List"))
-	assert.True(t, invoked)
-
-	invoked = false
-	recorder.submitMethodInvocation("foo", "List", invoker)
-	assert.False(t, invoked)
 
 	// Test a different method
-	invoked = false
 	assert.False(t, recorder.methodInvoked("foo", "Exec"))
-	recorder.submitMethodInvocation("foo", "Exec", invoker)
+	recorder.recordMethodInvocation("foo", "Exec")
 	assert.True(t, recorder.methodInvoked("foo", "Exec"))
-	assert.True(t, invoked)
-}
-
-func TestRecorder_RecordsMethodInvocationsOnce(t *testing.T) {
-	recorder := newRecorder()
-	var count int
-	invoker := func() { count++ }
-	var wg sync.WaitGroup
-	for i := 0; i <= 100; i++ {
-		wg.Add(1)
-		go func() {
-			recorder.submitMethodInvocation("foo", "Read", invoker)
-			wg.Done()
-		}()
-	}
-	wg.Wait()
-	assert.Equal(t, 1, count)
-	assert.True(t, recorder.methodInvoked("foo", "Read"))
 }
diff --git a/plugin/gcp/computeInst_test.go b/plugin/gcp/computeInst_test.go
index 8b94a3a..0bc2263 100644
--- a/plugin/gcp/computeInst_test.go
+++ b/plugin/gcp/computeInst_test.go
@@ -7,7 +7,6 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
-	"time"
 
 	"github.com/puppetlabs/wash/plugin"
 	"github.com/stretchr/testify/assert"
@@ -15,7 +14,7 @@ import (
 )
 
 func TestComputeInstance(t *testing.T) {
-	inst := compute.Instance{Name: "foo", CreationTimestamp: time.Now().Format(time.RFC3339) }
+	inst := compute.Instance{Name: "foo"}
 	compInst := newComputeInstance(&inst, computeProjectService{})
 	assert.Equal(t, "foo", compInst.Name())
 	assert.Implements(t, (*plugin.Parent)(nil), compInst)