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

make secret update support name and id prefix #30856

Merged
merged 1 commit into from Mar 8, 2017

Conversation

@allencloud
Contributor

allencloud commented Feb 9, 2017

Signed-off-by: allencloud allen.sun@daocloud.io

This PR makes daemon support update secret by receiving secret's name. name prefix, id, and id prefix.

- What I did

  1. when updating secret in daemon, daemon gets the secret first by name,id( prefix), then update the secret.

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@AkihiroSuda

This comment has been minimized.

Show comment
Hide comment
@AkihiroSuda

AkihiroSuda Feb 9, 2017

Member

I think we need an IT for this.

Member

AkihiroSuda commented Feb 9, 2017

I think we need an IT for this.

@allencloud

This comment has been minimized.

Show comment
Hide comment
@allencloud

allencloud Feb 9, 2017

Contributor

Thanks for your feedback. @AkihiroSuda
While could I ask that what does IT mean?

Contributor

allencloud commented Feb 9, 2017

Thanks for your feedback. @AkihiroSuda
While could I ask that what does IT mean?

@AkihiroSuda

This comment has been minimized.

Show comment
Hide comment
@AkihiroSuda

AkihiroSuda Feb 9, 2017

Member

"integration test" 😄

Member

AkihiroSuda commented Feb 9, 2017

"integration test" 😄

@allencloud

This comment has been minimized.

Show comment
Hide comment
@allencloud

allencloud Feb 9, 2017

Contributor

Totally reasonable. Thanks for your advice. While when I begin to code the test case, I found that almost all the api swarm test cases are located in one file docker_api_swarm_test.go including node test, service test, swarm test, task test. The big file makes me feel difficult to write test case and I think it will definitely make it hard to maintain.
Then I hope to split the docker_api_swarm_test.go into several test files docker_api_swarm_service_test.go, docker_api_swarm_node_test.go, docker_api_swarm_secret_test.go and so on.
After that I come to continue this PR with the integration test. How about that? @AkihiroSuda
see #30871

Contributor

allencloud commented Feb 9, 2017

Totally reasonable. Thanks for your advice. While when I begin to code the test case, I found that almost all the api swarm test cases are located in one file docker_api_swarm_test.go including node test, service test, swarm test, task test. The big file makes me feel difficult to write test case and I think it will definitely make it hard to maintain.
Then I hope to split the docker_api_swarm_test.go into several test files docker_api_swarm_service_test.go, docker_api_swarm_node_test.go, docker_api_swarm_secret_test.go and so on.
After that I come to continue this PR with the integration test. How about that? @AkihiroSuda
see #30871

@vdemeester

This comment has been minimized.

Show comment
Hide comment
@vdemeester
Member

vdemeester commented Feb 13, 2017

@diogomonica

This comment has been minimized.

Show comment
Hide comment
@diogomonica

diogomonica Feb 13, 2017

Contributor

This seems fine.

Contributor

diogomonica commented Feb 13, 2017

This seems fine.

@allencloud

This comment has been minimized.

Show comment
Hide comment
@allencloud

allencloud Feb 20, 2017

Contributor

ping @ehazlett @thaJeztah
Could you help review this one? ^^
thanks a lot.

Contributor

allencloud commented Feb 20, 2017

ping @ehazlett @thaJeztah
Could you help review this one? ^^
thanks a lot.

@allencloud

This comment has been minimized.

Show comment
Hide comment
@allencloud

allencloud Feb 28, 2017

Contributor

Any update, my friend? ^^
ping @AkihiroSuda @vdemeester @tiborvass @thaJeztah

Contributor

allencloud commented Feb 28, 2017

Any update, my friend? ^^
ping @AkihiroSuda @vdemeester @tiborvass @thaJeztah

@@ -370,7 +373,19 @@ func (d *Swarm) DeleteSecret(c *check.C, id string) {
c.Assert(status, checker.Equals, http.StatusNoContent, check.Commentf("output: %q", string(out)))
}
// GetSwarm return the current swarm object
// UpdateSecret updates the swarm secret identified by the specified id

This comment has been minimized.

@thaJeztah

thaJeztah Feb 28, 2017

Member

Should we mention here that only labels can be updated?

@thaJeztah

thaJeztah Feb 28, 2017

Member

Should we mention here that only labels can be updated?

This comment has been minimized.

@allencloud

allencloud Mar 3, 2017

Contributor

updated, thanks

@allencloud

allencloud Mar 3, 2017

Contributor

updated, thanks

"test": "test1",
},
},
[]byte("TESTINGDATA"),

This comment has been minimized.

@thaJeztah

thaJeztah Feb 28, 2017

Member

Perhaps we need a test that updating the secret data produces an error

@thaJeztah

thaJeztah Feb 28, 2017

Member

Perhaps we need a test that updating the secret data produces an error

This comment has been minimized.

@allencloud

allencloud Mar 3, 2017

Contributor

updated, PTAL

@allencloud

allencloud Mar 3, 2017

Contributor

updated, PTAL

@allencloud

This comment has been minimized.

Show comment
Hide comment
@allencloud

allencloud Mar 6, 2017

Contributor

Could you help to review this pr once more? Thanks a lot. @AkihiroSuda @vdemeester @diogomonica @thaJeztah

Contributor

allencloud commented Mar 6, 2017

Could you help to review this pr once more? Thanks a lot. @AkihiroSuda @vdemeester @diogomonica @thaJeztah

make secret update support name and id prefix
Signed-off-by: allencloud <allen.sun@daocloud.io>
@thaJeztah

LGTM. Thanks for updating the test

@vdemeester

LGTM 🐸

@vdemeester vdemeester merged commit d4a8c3b into moby:master Mar 8, 2017

4 of 6 checks passed

powerpc Jenkins build Docker-PRs-powerpc 174 has failed
Details
z Jenkins build Docker-PRs-z 62 has failed
Details
dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 31393 has succeeded
Details
janky Jenkins build Docker-PRs 40011 has succeeded
Details
windowsRS1 Jenkins build Docker-PRs-WoW-RS1 11082 has succeeded
Details

@GordonTheTurtle GordonTheTurtle added this to the 17.04.0 milestone Mar 8, 2017

@allencloud allencloud deleted the allencloud:make-secret-update-support-name-and-id-prefix branch Mar 8, 2017

@mistyhacks

This comment has been minimized.

Show comment
Hide comment
@mistyhacks

mistyhacks Mar 17, 2017

Contributor

Does this also apply to listing and inspecting the secret?

Contributor

mistyhacks commented Mar 17, 2017

Does this also apply to listing and inspecting the secret?

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