cmd/build-extensions-container: use cosa meta to update meta.json #3152
Conversation
|
For some reason I was thinking there would be a way for us to pass in just the new json (for the extensions container) and have that all get merged properly. I think what we have here has the same problems as before (i.e. we read the Maybe the |
Yup, that's correct. See the |
cmd/build-extensions-container.go
Outdated
| @@ -68,7 +68,7 @@ func buildExtensionContainer() error { | |||
| metapath := filepath.Join(buildPath, "meta.json") | |||
There was a problem hiding this comment.
instead of reading in the meta.json here and defining a new var cosaBuild cosa.Build below, could we just use the lastBuild that we already have from the cosa.ReadBuild() call at the beginning of this function?
There was a problem hiding this comment.
if we do do that I wouldn't mind renaming lastBuild to cosaBuild above and using that variable name.
There was a problem hiding this comment.
Thanks for implementing this. I do realize now though that there is a lot more time that passes from the time we read the build (and current meta.json) and we write it out. I think that's OK, just wanted to call it out in case this raises any alarm bells with @jlebon.
👍 - so if we update |
jmarrero
force-pushed
the
lock-meta
branch
2 times, most recently
from
31cd31a
to
e69965c
Compare
cmd/build-extensions-container.go
Outdated
| cosaBuild.BuildArtifacts.ExtensionsContainer = &cosa.Artifact{ | ||
| Path: targetname, | ||
| Sha256: sha256sum, | ||
| SizeInBytes: float64(stat.Size()), | ||
| SkipCompression: true, | ||
| } | ||
| cosaBuild.MetaStamp = cosaBuild.MetaStamp + 1 |
There was a problem hiding this comment.
@jlebon is this safe or should we go with the current time?
There was a problem hiding this comment.
It'd still work for our purposes, but yeah ideally it should be the current time. This should work:
| cosaBuild.MetaStamp = cosaBuild.MetaStamp + 1 | |
| cosaBuild.MetaStamp = time.Now().UnixNano() |
cmd/build-extensions-container.go
Outdated
| cosaBuild.BuildArtifacts.ExtensionsContainer = &cosa.Artifact{ | ||
| Path: targetname, | ||
| Sha256: sha256sum, | ||
| SizeInBytes: float64(stat.Size()), | ||
| SkipCompression: true, | ||
| } | ||
| cosaBuild.MetaStamp = cosaBuild.MetaStamp + 1 |
There was a problem hiding this comment.
It'd still work for our purposes, but yeah ideally it should be the current time. This should work:
| cosaBuild.MetaStamp = cosaBuild.MetaStamp + 1 | |
| cosaBuild.MetaStamp = time.Now().UnixNano() |
|
I guess we've been taking a hit here in blazing a trail in using Go for cosa build stuff. Hopefully it will pay itself back later by writing less shell script (and involving less dynamic python tracebacks)... |
|
/test rhcos |
|
|
||
| err = ioutil.WriteFile(metapath, newBytes, 0644) | ||
| extensions_container_meta_path := filepath.Join(buildPath, "meta.extensions-container.json") | ||
| err = ioutil.WriteFile(extensions_container_meta_path, newBytes, 0644) | ||
| if err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
I think we're missing a defer here to delete that file, no?
|
Related to sharing locking; I think it would make sense to do the lowest common denominator of forking off |
closes #3149