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

Feature/docker.volume #453

Merged
merged 8 commits into from Nov 7, 2016

Conversation

Projects
None yet
2 participants
@ryane
Copy link
Contributor

ryane commented Oct 31, 2016

fixes #299

@ryane ryane added the enhancement label Oct 31, 2016

@ryane ryane added this to the 0.4.0 milestone Oct 31, 2016

@stevendborrelli

This comment has been minimized.

Copy link
Member

stevendborrelli commented Nov 4, 2016

Getting a panic when I try to run the sample:

./converge plan --local samples/dockerVolume.hcl
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x30 pc=0x421382]

goroutine 22 [running]:
panic(0x6308e0, 0xc4200120a0)
    /usr/local/Cellar/go/1.7.1/libexec/src/runtime/panic.go:500 +0x1a1
github.com/asteris-llc/converge/resource/docker/volume.(*Volume).Check(0xc420175080, 0xa10100, 0xc42015d900, 0x20, 0xc42015d900, 0x0, 0x0)
    /Users/steve/go/src/github.com/asteris-llc/converge/resource/docker/volume/volume.go:65 +0x3c2

ryane added some commits Oct 28, 2016

@ryane ryane force-pushed the feature/docker.volume branch from 18cfa6f to d8c4943 Nov 4, 2016

@ryane

This comment has been minimized.

Copy link
Contributor

ryane commented Nov 4, 2016

panic should be fixed + rebased

if v.State == StatePresent && vol != nil && v.Force {
expectedLabels := mapToString(v.Labels)
actualLabels := mapToString(vol.Labels)
v.Status.AddDifference("labels", actualLabels, expectedLabels, "")

This comment has been minimized.

@stevendborrelli

stevendborrelli Nov 4, 2016

Member

Shouldn't we only add these diffs if they don't match?

This comment has been minimized.

@ryane

ryane Nov 4, 2016

Contributor

it's an option but this approach is consistent with the existing docker resources. this is discussed more in #325. let me know if you think I should change it.

we could also update the human printer to only print diffs with changes. something like:

diff --git a/prettyprinters/human/human.go b/prettyprinters/human/human.go
index 74d41ac..4e137a8 100644
--- a/prettyprinters/human/human.go
+++ b/prettyprinters/human/human.go
@@ -136,7 +136,9 @@ func (p *Printer) DrawNode(g *graph.Graph, id string) (pp.Renderable, error) {
    Has Changes: {{if .HasChanges}}{{yellow "yes"}}{{else}}no{{end}}
    Changes:
        {{- range $key, $values := .Changes}}
+       {{- if $values.Changes}}
        {{cyan $key}}:  {{diff ($values.Original) ($values.Current)}}
+       {{- end}}
        {{- else}} No changes {{- end}}

 `)

@stevendborrelli stevendborrelli merged commit 30faed9 into master Nov 7, 2016

2 checks passed

codeclimate Code Climate didn't find any new or fixed issues.
Details
wercker/build Wercker pipeline passed
Details

@stevendborrelli stevendborrelli deleted the feature/docker.volume branch Nov 7, 2016

BrianHicks pushed a commit that referenced this pull request Dec 22, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment