feature: add spec annotation flag #1046
Conversation
|
Use this specify flag make the previous steps more simple.
|
Codecov Report
@@ Coverage Diff @@
## master #1046 +/- ##
==========================================
- Coverage 15.68% 15.64% -0.04%
==========================================
Files 139 139
Lines 8499 8518 +19
==========================================
Hits 1333 1333
- Misses 7066 7085 +19
Partials 100 100
Continue to review full report at Codecov.
|
allencloud
changed the title
|
LGTM |
pouchrobot
added
the
LGTM
apis/types/container_config.go
Outdated
| @@ -108,6 +108,9 @@ type ContainerConfig struct { | |||
|
|
|||
| // The working directory for commands to run in. | |||
| WorkingDir string `json:"WorkingDir,omitempty"` | |||
|
|
|||
| // Annotations send to runtime spec. | |||
| SpecAnnotation map[string]string `json:"SpecAnnotation,omitempty"` | |||
There was a problem hiding this comment.
Please add this in swagger.yml.
|
do we need show |
Yes, I think so. @Ace-Tang |
|
Please add some unit test for the function validateAnnotation and add integration test for the field. |
apis/swagger.yml
Outdated
| @@ -1751,6 +1751,11 @@ definitions: | |||
| x-nullable: true | |||
| additionalProperties: | |||
| type: "string" | |||
| SpecAnnotation: | |||
| description: "nnotations send to runtime spec." | |||
There was a problem hiding this comment.
s/nnotations/annotations ?
|
@allencloud @rudyfly , has updated. About |
| res.Assert(c, icmd.Success) | ||
| defer DelContainerForceMultyTime(c, name) | ||
|
|
||
| command.PouchRun("start", name).Assert(c, icmd.Success) |
There was a problem hiding this comment.
I think after start, we need to check these annotation as well with pouch inspect.
In addition , I think we need to inspect after create, and then start
There was a problem hiding this comment.
this has been done in test/cli_run_test.go
There was a problem hiding this comment.
No, I do not think they are the same. They have called the same API, but in cli side, they called different code. Double check would be nicer to guarantee the code quality.
There was a problem hiding this comment.
Follow the previous logic, inspect test in doing in cli_run_test.go, not in start test, as I agree with you, this also related all test case in test/cli_start_test.go.
There was a problem hiding this comment.
oh! I miss create test, this things should be done there too.
There was a problem hiding this comment.
updated. @allencloud
|
This pr will be related to pr #1041. If this is merged first, #1041 needs rebased. @HusterWan |
|
got it |
spec annotation flag pass all user defined runtime annotation to runtime, note that whether the annotation will take effect depend on your runtime inplement. Signed-off-by: Ace-Tang <aceapril@126.com>
|
LGTM, too. |
spec annotation flag pass all user defined runtime annotation to
runtime, note that whether the annotation will take effect depend
on your runtime inplement.
Signed-off-by: Ace-Tang aceapril@126.com
Ⅰ. Describe what this PR did
Ⅱ. Does this pull request fix one issue?
Ⅲ. Describe how you did it
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews