Netlify docs deployment

[tjovicic]

After lessons learned in #805, this is 2nd version of new docs deployment.

[netlify]

✔️ Deploy Preview for devel-docs-dagger-io ready!

🔨 Explore the source changes: 9e2daf3

🔍 Inspect the deploy log: https://app.netlify.com/sites/devel-docs-dagger-io/deploys/60ed613a90e9290007032399

😎 Browse the preview: https://deploy-preview-812--devel-docs-dagger-io.netlify.app

[aluzzardi]

Looks good :)

Is it ready to be merged or should we wait for @samalba to fix the netlify dependency installation and retry this PR with that?

[aluzzardi]

did you mean to include the docs-tiho environment in the PR?

[tjovicic]

Ou, i haven't. I'll remove it.

[tjovicic]

Looks good :)

Is it ready to be merged or should we wait for @samalba to fix the netlify dependency installation and retry this PR with that?

We should fix the dependency installation before merging

[samalba]

We should fix the dependency installation before merging

I chatted briefly with @aluzzardi yesterday about this. It seems that the best solution is to include netlify's build image - however it seems to be over 800MB of layers... Not sure if there is a better way, I did not dig into it that much.

@tjovicic do you mind looking into this?

[tjovicic]

We should fix the dependency installation before merging

I chatted briefly with @aluzzardi yesterday about this. It seems that the best solution is to include netlify's build image - however it seems to be over 800MB of layers... Not sure if there is a better way, I did not dig into it that much.

@tjovicic do you mind looking into this?

@samalba Sure, no problem. I can look into that.

[aluzzardi]

CI failure looks legit -- maybe there's a regression due to the upgrade of the netlify CLI

[tjovicic]

CI failure looks legit -- maybe there's a regression due to the upgrade of the netlify CLI

The

CI failure looks legit -- maybe there's a regression due to the upgrade of the netlify CLI

@aluzzardi I'm not sure, i've reverted the image bump change and it's still failing

[tjovicic]

We should fix the dependency installation before merging

I chatted briefly with @aluzzardi yesterday about this. It seems that the best solution is to include netlify's build image - however it seems to be over 800MB of layers... Not sure if there is a better way, I did not dig into it that much.

@tjovicic do you mind looking into this?

@samalba I've done some digging and there is no better way than including that netlify image. If we want to support npm, ruby, python... then I expect a giant image. But I'm not sure if it's worth it. Maybe just let user do the build part as a separate dagger step?

[aluzzardi]

@samalba I've done some digging and there is no better way than including that netlify image. If we want to support npm, ruby, python... then I expect a giant image. But I'm not sure if it's worth it. Maybe just let user do the build part as a separate dagger step?

I hear the downside (big image, it will be quite slow on first dagger up), but I think it's necessary to guarantee that building a Netlify site on Dagger yields the same result as without Dagger.

I see 3 options:

1. Move as is. The downside is we have to download a big image. Hopefully investing in caching will mimimize this issue.

2. Separate Build and Publish. We could provide a way to optionally build (which will use the netlify image) and then push using an alpine based image.

To do this, we could either provide 2 different components (e.g. netlify.#Build and netlify.#Site) or having a build: *true | false option in netlify.#Site (which would add an extra netlify build step using the netlify image).

This could be a good option, however, I think it's weird to use netlify without building with netlify (e.g. you get a different website depending on whether you build on your own or through netlify).

3. Shrink the netlify image. We could create our own alpine-based netlify image which uses the same entrypoint shell scripts but tries to shrink the image size. I think this is a lot of work and could result in subtle differences, hard to debug.

[aluzzardi]

@aluzzardi I'm not sure, i've reverted the image bump change and it's still failing

# 8:44AM INF TestNetlify.check | #16 0.346 curl: try 'curl --help' or 'curl --manual' for more information
# 8:44AM ERR TestNetlify.check | failed: process "/bin/bash --noprofile --norc -eo pipefail -c test \"$(curl )\" = \"gkrljymvmtvd\"" did not complete successfully: exit code: 1    duration=1.9s
# 8:44AM FTL system | failed to up environment: task failed: process "/bin/bash --noprofile --norc -eo pipefail -c test \"$(curl )\" = \"gkrljymvmtvd\"" did not complete successfully: exit code: 1

The problem seems to be here: test \"$(curl )\" = \"gkrljymvmtvd\""

That's what we do in stdlib/netlify/tests/netlify.cue: test "$(curl \#(deploy.deployUrl))" = "\#(data.out)"

Looks like deploy.deployUrl is empty.

[tjovicic]

@aluzzardi I'm not sure, i've reverted the image bump change and it's still failing

# 8:44AM INF TestNetlify.check | #16 0.346 curl: try 'curl --help' or 'curl --manual' for more information
# 8:44AM ERR TestNetlify.check | failed: process "/bin/bash --noprofile --norc -eo pipefail -c test \"$(curl )\" = \"gkrljymvmtvd\"" did not complete successfully: exit code: 1    duration=1.9s
# 8:44AM FTL system | failed to up environment: task failed: process "/bin/bash --noprofile --norc -eo pipefail -c test \"$(curl )\" = \"gkrljymvmtvd\"" did not complete successfully: exit code: 1

The problem seems to be here: test \"$(curl )\" = \"gkrljymvmtvd\""

That's what we do in stdlib/netlify/tests/netlify.cue: test "$(curl \#(deploy.deployUrl))" = "\#(data.out)"

Looks like deploy.deployUrl is empty.

That's weird because I haven't touch the tests or the netlify package...

[tjovicic]

@samalba I've done some digging and there is no better way than including that netlify image. If we want to support npm, ruby, python... then I expect a giant image. But I'm not sure if it's worth it. Maybe just let user do the build part as a separate dagger step?

I hear the downside (big image, it will be quite slow on first dagger up), but I think it's necessary to guarantee that building a Netlify site on Dagger yields the same result as without Dagger.

I see 3 options:

1. Move as is. The downside is we have to download a big image. Hopefully investing in caching will mimimize this issue.
2. Separate Build and Publish. We could provide a way to optionally build (which will use the netlify image) and then push using an alpine based image.

To do this, we could either provide 2 different components (e.g. netlify.#Build and netlify.#Site) or having a build: *true | false option in netlify.#Site (which would add an extra netlify build step using the netlify image).

This could be a good option, however, I think it's weird to use netlify without building with netlify (e.g. you get a different website depending on whether you build on your own or through netlify).

1. Shrink the netlify image. We could create our own alpine-based netlify image which uses the same entrypoint shell scripts but tries to shrink the image size. I think this is a lot of work and could result in subtle differences, hard to debug.

I understand. I wasn't a Netlify user before but now I understand that the build step is a popular feature among users. I can continue on with adding the build-image to our package.

[aluzzardi]

That's weird because I haven't touch the tests or the netlify package...

Yeah I don't get it either. Want me to take a look?

[aluzzardi]

I understand. I wasn't a Netlify user before but now I understand that the build step is a popular feature among users. I can continue on with adding the build-image to our package.

Yeah, basically it's pretty much impossible to set up a Netlify project without build (unless you go through the API).

So we have the assume the starting point is someone using Netlify alongside Netlify Build, wanting to move the process over to Dagger (for instance to have better control on when to deploy, like in this PR we want to deploy on every release).

@samalba Any opinion on the 3 options (asking since you've implemented the build support)?

[samalba]

I would go with option 2. Although it's not possible to deploy without build using the GUI. It's possible using the CLI. And it's also a valid use case. That way, if I don't rely on netlify.#Build, I don't need to pull the fat image. I can also swap out netlify.#Build with yarn.#Package if I don't need netlify-specific things.

I think option 3 is a no-go given the extra maintenance work.

Option 1 is ok.

[tjovicic]

That's weird because I haven't touch the tests or the netlify package...

Yeah I don't get it either. Want me to take a look?

Yea, can you help me with that?

[tjovicic]

I would go with option 2. Although it's not possible to deploy without build using the GUI. It's possible using the CLI. And it's also a valid use case. That way, if I don't rely on netlify.#Build, I don't need to pull the fat image. I can also swap out netlify.#Build with yarn.#Package if I don't need netlify-specific things.

I think option 3 is a no-go given the extra maintenance work.

Option 1 is ok.

After Sam has explained it so clearly, I would prefer option 2 also.

[aluzzardi]

Yea, can you help me with that?

Sure, will take a look

After Sam has explained it so clearly, I would prefer option 2 also.

Let's do that then

[aluzzardi]

This seems to be the problem that makes CI fail. I don't know why but putting --dir back fixes the issue.

[tjovicic]

@aluzzardi @samalba I've tried to create Build step using Netlify's build-image but I encountered 2 problems:

• Their run-build,sh script should be updated with a base directory env variable: https://github.com/netlify/build-image/blob/653805ca4a64301556e56dc4b321ef8fc20cbb7c/run-build.sh#L19. So we can set our website directory as the starting point when installing dependencies
• Currently netlify-cli isn't provided with the image. When I try to install it using:

source ~/.nvm/nvm.sh  && nvm install 12 && npm install netlify-cli -g

and then try to run

/opt/build-bin/build netlify build  

I get

Executing user command: netlify build
/opt/build-bin/build: line 37: netlify: command not found

I think they are either doing something with bash sessions or messing with $PATH.

I've been doing this task for some time and the only way I see supporting Netlify build step is to have our own fork. What do you think?

[aluzzardi]

Their run-build,sh script should be updated with a base directory env variable: https://github.com/netlify/build-image/blob/653805ca4a64301556e56dc4b321ef8fc20cbb7c/run-build.sh#L19. So we can set our website directory as the starting point when installing dependencies

This should be doable on our end, something like mount: "/src": from source then env: NETLIFY_REPO_DIR: "/src"

source ~/.nvm/nvm.sh && nvm install 12 && npm install netlify-cli -g

https://github.com/netlify/build-image/blob/653805ca4a64301556e56dc4b321ef8fc20cbb7c/run-build.sh#L28

I see they're already setting up yarn, we should be able to keep our same setup section: https://github.com/dagger/dagger/blob/main/stdlib/netlify/netlify.cue#L70

Basically something like this (taking the existing code with some small tweaks):

	ctr: os.#Container & {
		image: docker.#Pull & {
                        from: "FIXME netlify build image url"
		}
		setup: [
			"yarn global add netlify-cli@3.38.10",
		]
		env: {
			NETLIFY_REPO_DIR: "/src"
		}
                command: "FIXME"
		dir: "/src"
		mount: "/src": from: contents
	}

So the only remaining part is Executing user command: netlify build. I think we can get past that, somehow.

Have you tried running the container manually, checking out PATH and doing the yarn global install to see where the binaries go?

We could either run the command with the full path, or inject a custom PATH, or something else

[aluzzardi]

Regarding the PATH issue, I think the root problem might be because os.#Container overrides $PATH with its own thing:

https://github.com/dagger/dagger/blob/main/stdlib/os/container.cue#L84

Which effectively overrides whatever PATH is defined by the container we use.

@shykes Do you remember what was the reason behind override PATH?

[shykes]

Regarding the PATH issue, I think the root problem might be because os.#Container overrides $PATH with its own thing:

https://github.com/dagger/dagger/blob/main/stdlib/os/container.cue#L84

Which effectively overrides whatever PATH is defined by the container we use.

@shykes Do you remember what was the reason behind override PATH?

At the time, Dagger didn’t preserve metadata when pulling images, so I had to manually set PATH. Initially I did it with env.PATH directly, but it’s not composable: you can’t start from an image with its own PATH, and add extra search component. That problem is solved by a Cue map, with the final PATH env variable generated at the end.

I think we should keep that API, and either:

1. Improve the fetch-container implementation so that it exposes the docker image metadata in cue somewhere. Then os.#Container could take it into account and it wouldn’t get overridden. (cleanest approach; more work)

2. Change the default value of in os.#Container.shell.search so that it is empty (ie. don’t override PATH by default). (short term fix; less work)

[aluzzardi]

Initially I did it with env.PATH directly, but it’s not composable: you can’t start from an image with its own PATH, and add extra search component

I think this breaks PATH, even if we took into account the metadata from fetchcontainer. Images deal with PATH in various ways, could be with ENV instructions, could be in the entrypoint or bashrc or other means. I think we should leave PATH alone and let the container do its own thing. If someone wants to append something to the path, the safest way is to do an export PATH=$PATH:/foo within the script itself.

[shykes]

I think that sets a bad precedent (“use Cue to configure all the things! Except this thing, don’t touch that”) but won’t argue any further. I’m fine either way. Also I don’t have a pressing need for it, and there is a pressing need to fix this issue, so we can always reopen later when/if someone asks for it.

[samalba]

Closing this as it's stale.