Added saving command to annotation feature
#733
Conversation
|
@surajnarwade, thank you for the pull request! We'll ping some people to review your PR. @procrypt and @cdrage, please review this. |
k8s-ci-robot
added
the
cncf-cla: yes
|
Couple of files have been added by accident and need to be deleted: |
|
Can you please also separate the commits so we can review this easier (one for code change, other for test changes) |
surajnarwade
force-pushed
the
save-cmd
branch
3 times, most recently
from
2132f6e to
13d68fa
Compare
| @@ -32,6 +32,7 @@ import ( | |||
| ) | |||
|
|
|||
| func newServiceConfig() kobject.ServiceConfig { | |||
| fmt.Println() | |||
| } | ||
| //if !equalStringMaps(config.Annotations, meta.Annotations) { | ||
| // return fmt.Errorf("Found different annotations: %#v vs. %#v", config.Annotations, meta.Annotations) | ||
| //} |
There was a problem hiding this comment.
I think we should keep it, as it's test for annotation, but it's big blocker for test so I commented it
There was a problem hiding this comment.
Okay, maybe re-adding it back would be a good idea or pushing it to a separate test.
There was a problem hiding this comment.
@surajnarwade create an issue for tracking it, otherwise it will just slip from everyone's mind
| @@ -114,7 +116,14 @@ func ConfigAnnotations(service kobject.ServiceConfig) map[string]string { | |||
| for key, value := range service.Annotations { | |||
| annotations[key] = value | |||
| } | |||
| annotations["kompose.cmd"] = strings.Join(os.Args, " ") | |||
| version := exec.Command("kompose", "version") | |||
There was a problem hiding this comment.
I don't like how we have an exec command here... but I don't think there's an alternative way to do this getting the actual Git hash...
There was a problem hiding this comment.
no option for that as of now
|
Let's wait until @kadel get's back for him to review this. But other than uncommenting those tests / separating them, LGTM. |
|
@kadel , needs your review here |
|
@surajnarwade also add |
|
@surajnarwade this one #733 (comment) |
|
DO NOT MERGE YET. @surajnarwade Please update the commit message with a description of what has been implemented, including an example. It is still blank. |
This command will add `kompose command` used to generate artifacts as well as `kompose version`,
for ex,
```
metadata:
annotations:
kompose.cmd: kompose convert -f /home/snarwade --stdout
kompose.version: 1.0.0 (HEAD)
```
For functional test, Now each test has template like,
```
"annotations": {
"kompose.cmd": "%CMD%",
"kompose.version": "%VERSION%"
```
Because, for every machine these values will be different.
Updated functional test with new annotations
|
@cdrage updated commit message |
|
Thank you @surajnarwade ! Mergin :) |
This will save kompose command and version to annoatations, which were used to generate artifacts.
Fixes #639