Skip to content
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

Add volumes to images #1465

Merged
merged 8 commits into from May 6, 2021
Merged

Add volumes to images #1465

merged 8 commits into from May 6, 2021

Conversation

jLopezbarb
Copy link
Contributor

Signed-off-by: Javier López Barba javier@okteto.com

Fixes #1439

Proposed changes

  • Build an image if there are volumes that are necessary to run the application (nginx config file)

Signed-off-by: Javier López Barba <javier@okteto.com>
@codecov
Copy link

codecov bot commented Apr 30, 2021

Codecov Report

Merging #1465 (e86edfa) into master (9f0f7d9) will decrease coverage by 0.19%.
The diff coverage is 17.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1465      +/-   ##
==========================================
- Coverage   36.19%   36.00%   -0.20%     
==========================================
  Files          80       80              
  Lines        7689     7744      +55     
==========================================
+ Hits         2783     2788       +5     
- Misses       4551     4601      +50     
  Partials      355      355              
Impacted Files Coverage Δ
pkg/cmd/stack/translate.go 68.56% <0.00%> (-6.10%) ⬇️
pkg/model/stack.go 44.35% <0.00%> (ø)
pkg/registry/file.go 16.94% <0.00%> (-6.87%) ⬇️
pkg/model/stack_serializer.go 47.78% <100.00%> (+0.55%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9f0f7d9...e86edfa. Read the comment docs.


if !building && forceBuild {
log.Warning("Ignoring '--build' argument. There are not 'build' primitives in your stack")
func addVolumeMountsToSvcs(ctx context.Context, s *model.Stack, buildKitHost string, isOktetoCluster, forceBuild, noCache bool) (bool, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
func addVolumeMountsToSvcs(ctx context.Context, s *model.Stack, buildKitHost string, isOktetoCluster, forceBuild, noCache bool) (bool, error) {
func addVolumeMountsToBuiltImage(ctx context.Context, s *model.Stack, buildKitHost string, isOktetoCluster, forceBuild, noCache bool) (bool, error) {

if len(svc.VolumeMounts) != 0 {
if !hasAddedAnyVolumeMounts {
hasAddedAnyVolumeMounts = true
log.Information("Running your build in %s...", buildKitHost)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check that hasBuiltSomething is false?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That checks if a previous service has created any mount volume on other image, so it only gets into it the first time

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeap, but if hasBuiltSomething is true, this has been already printed, not?
the idea is to show the logs only on the first image been built

}
svc.Build = svcBuild
if isOktetoCluster && !strings.HasPrefix(svc.Image, "okteto.dev") {
tag := strings.Replace(svc.Image, ":", "-", 1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's always create a new image tag for this, for example:
okteto.dev/{stack_name}-{service_name}:okteto-with-volume-mounts

tag := strings.Replace(svc.Image, ":", "-", 1)
svc.Image = fmt.Sprintf("okteto.dev/%s:okteto", tag)
}
log.Information("Building image for service '%s'...", name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
log.Information("Building image for service '%s'...", name)
log.Information("Building image for service '%s' to include host volumes...", name)

Signed-off-by: Javier López Barba <javier@okteto.com>
Signed-off-by: Javier López Barba <javier@okteto.com>
Signed-off-by: Javier López Barba <javier@okteto.com>
Signed-off-by: Javier López Barba <javier@okteto.com>
Signed-off-by: Javier López Barba <javier@okteto.com>
Signed-off-by: Javier López Barba <javier@okteto.com>
Signed-off-by: Javier López Barba <javier@okteto.com>
@jLopezbarb jLopezbarb merged commit 01feb91 into master May 6, 2021
@jLopezbarb jLopezbarb deleted the jlopezbarb/add-volume-mounts branch May 6, 2021 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add mount volumes to the Dockerfile in Okteto Stacks
2 participants