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

feat: replace set-env-vars task with prepare-buildpack-build one #49

Merged
merged 2 commits into from
Nov 2, 2022

Conversation

kseniyashaydurova
Copy link
Contributor

@kseniyashaydurova kseniyashaydurova commented Oct 25, 2022

This change is needed in cases, when we have few different app stacks, which may be built with buildpacks (I need this change for https://saritasa.atlassian.net/browse/OVIO-66 to be able to build 2 separate .NET components - api and video processor from the same repo, both of them can be built with buildpacks).

Added new TriggerBinding params, which may be customized during build process buildpack_config_filename and project_config_filename (they allow to define custom names for buildpack.yml and project.toml files) + reorganized set-env-vars buildpack task to add one more prepare step, which changes corresponding project_config_filename and buildpack_config_filename files to buildpack.yml and project.toml files.

So the flow is following:

  1. We add different buildpack.yml and project.toml files for each component, i.e. ovio-api-project.toml, ovio-video-processor-project.toml, ovio-api-buildpack.yml, ovio-video-processor-buildpack.yml instead of original buildpack.yml and project.toml ones.
  2. When we add project in tekton section in EKS, we need to define correct buildpack_config_filename and project_config_filename filenames and during build correct files would be taken and renamed to ordinary buildpack.yml and project.toml files.

That's how these changes are added to rocks EKS: https://github.com/saritasa-nest/saritasa-rocks-kubernetes-aws/pull/93

Copy link
Contributor

@dmitry-mightydevops dmitry-mightydevops left a comment

Choose a reason for hiding this comment

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

the project.toml file can specify the location of files that you need to build

[io.buildpacks]
include = [
    "cmd/",
    "go.mod",
    "go.sum",
    "*.go"
]

you can have different project.toml files in the repository and pass the descriptor as the argument

# build the app
pack build sample-app \
    --builder cnbs/sample-builder:bionic \
    --path  samples/apps/bash-script/ \
    --descriptor samples/apps/bash-script/new-project.toml

buildpack.yml is deprecated
https://paketo.io/docs/howto/dotnet-core/

so use project-COMPONENTNAME.toml to define the behavior. Also

[[build.env]]
name = "BPE_OVERRIDE_SERVER_PORT"
value = "8000"
[[build.buildpacks]]
uri = "java-maven"
[[build.buildpacks]]
uri = "paketo-buildpacks/environment-variables"

we can try using this buildpack
https://github.com/paketo-buildpacks/environment-variables in order to setup env vars

let's see if
/cnb/lifecycle/creator has argument to pass a custom named project.toml file.

that way you don't need to pre-rename the files into project.toml or buildpack.yml files.

@populov
Copy link
Contributor

populov commented Oct 26, 2022

let's see if
/cnb/lifecycle/creator has argument to pass a custom named project.toml file.

@dmitry-mightydevops no, it doesn't support project.toml. creator has argument -project, but it is not input file, it's an output file where creator writes information about generated project.

project.toml is for pack utility only.

populov
populov previously approved these changes Oct 26, 2022
@kseniyashaydurova
Copy link
Contributor Author

kseniyashaydurova commented Oct 31, 2022

@dmitry-mightydevops

the project.toml file can specify the location of files that you need to build

[io.buildpacks]
include = [
    "cmd/",
    "go.mod",
    "go.sum",
    "*.go"
]

you can have different project.toml files in the repository and pass the descriptor as the argument

# build the app
pack build sample-app \
    --builder cnbs/sample-builder:bionic \
    --path  samples/apps/bash-script/ \
    --descriptor samples/apps/bash-script/new-project.toml

Yes, it works in this way only when we build project locally with pack tool and I already configured Makefile to build different components in this way - https://github.com/saritasa-nest/ovio-backend/blob/develop/Makefile#L14

But it is not the way, how Tekton build works. We have there below command:

      /cnb/lifecycle/creator \
        -app=$(params.source_subpath) \
        -project-metadata=project.toml \
        -cache-dir=/cache \
        -layers=/layers \
        -platform=$(workspaces.source.path)/$(params.platform_dir) \
        -report=/layers/report.toml \
        -cache-image=$(params.cache_image) \
        -uid=$(params.user_id) \
        -gid=$(params.group_id) \
        -process-type=$(params.process_type) \
        -skip-restore=$(params.skip_restore) \
        -previous-image=$(resources.outputs.image.url) \
        -run-image=$(params.run_image) \
        $(resources.outputs.image.url)

So it has no descriptor parameter there to pass different project.toml file, we have only project-metadata parameter, but it is used just like metadata by Tekton, not like a building configuration, so Tekton doesn't support build configuration through project.toml file yet (buildpacks/tekton-integration#33)

@kseniyashaydurova
Copy link
Contributor Author

kseniyashaydurova commented Oct 31, 2022

@dmitry-mightydevops

pack.yml is deprecated https://paketo.io/docs/howto/dotnet-core/

It is not deprecated until v1.0.0 version (https://github.com/paketo-buildpacks/dotnet-core-runtime#configuration), latest version is v0.10.8. So we still can use it, anyway we set only project-path variable from there (later if buildpack.yml would become deprecated, we can set the same config with our current set-env-vars solution, which exports needed for dotnet-core buildpack variable BP_DOTNET_PROJECT_PATH):

dotnet-build:
  project-path: "Sources/Web/OvioApi"

@kseniyashaydurova
Copy link
Contributor Author

kseniyashaydurova commented Oct 31, 2022

@dmitry-mightydevops

let's see if /cnb/lifecycle/creator has argument to pass a custom named project.toml file.

that way you don't need to pre-rename the files into project.toml or buildpack.yml files.

I already checked that and no, we can't, Tekton not provides possibility to read build related configuration project.toml file because of cnb lifecycle usage. So current implementation is the only way to be able to build few components from the same repo.

If smth is not clear, let's discuss it in a call

@kseniyashaydurova
Copy link
Contributor Author

kseniyashaydurova commented Oct 31, 2022

@dmitry-mightydevops

we can try using this buildpack https://github.com/paketo-buildpacks/environment-variables in order to setup env vars

If you want to try to use it instead of our current set-env-vars solution, let's please create a separate task for it. But it will make us to add BPE_ related vars to all our project.toml files, instead of current simple env vars setting in project.toml:

[[ build.env ]]
    name = 'BP_DOTNET_PROJECT_PATH'
    value = 'Sources/Web/OvioApi'

But anyway, it looks like we can't avoid set-env-vars task even with this improvement, because Tekton and lifecycle not support reading info from project.toml file. So as for me it makes no sense

@DanilaKazakevich
Copy link
Contributor

Team, what do you think about switching tekton from /cnb/lifecycle/creator to pack?
Invoking creater has own benefits, but it seems pack allows us do more and also, developers will be able to build images on their own for local.

@kseniyashaydurova
Copy link
Contributor Author

@DanilaKazakevich discussed with @dmitry-mightydevops in Slack and decided to leave current solution for now with TODO mark to investigate this issue again, when Dmitry will upgrade Tekton engine and probably we will go in suggested by you way

@kseniyashaydurova kseniyashaydurova force-pushed the feature/support-few-buildpack-files branch from 45594ff to a5d0624 Compare November 1, 2022 06:13
@kseniyashaydurova kseniyashaydurova merged commit 5c3adcc into main Nov 2, 2022
@populov populov deleted the feature/support-few-buildpack-files branch November 3, 2022 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Needs Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants