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

Make action fully equivalent to the okteto build command #18

Merged
merged 10 commits into from
Sep 13, 2023

Conversation

teresaromero
Copy link
Contributor

Resolves https://github.com/okteto/app/issues/6884

In order to make the action fully equivalent to the okteto build command here are the following changes:

  • recover at entrypoint.sh the inputs documented at action.yml: tag,path and buildargs
  • remove unused global from both entrypoint.sh and action.yml. Users can push to global registry by specific tag.
  • include additional options no-cache, cache-from, export-cache, secret to be able to used them at the action
  • review Readme and add more examples of use

Tests done

Used repository https://github.com/okteto/go-getting-started and https://github.com/okteto/movies-with-helm to have both monorepo and single repo

As entrypoint.sh is a script, locally simulating the input from the action with running the script with arguments

  • Build single service not included at Okteto Manifest ./entrypoint.sh "myapp:dev" "Dockerfile" "."
  • Build single service included at Okteto Manifest ./entrypoint.sh "" "" "myapp"
  • Build all services included at Okteto Manifest ./entrypoint.sh "" "" ""

@jLopezbarb
Copy link
Contributor

Maybe we should include an action test for a Dockerfile in okteto/okteto repo


### `global`
Copy link
Contributor

Choose a reason for hiding this comment

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

If we remove global we may break existing workflows, should we release this as a breaking change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

global param is deprecated, the CLI does not use it. i think what should be done is to file an issue to remove this from the options available at CLI.

@teresaromero
Copy link
Contributor Author

Maybe we should include an action test for a Dockerfile in okteto/okteto repo

I was reviewing and this https://github.com/okteto/okteto/blob/master/integration/actions/build_test.go#L71 is already in place. Are you referring to other kind of tests?

I was thinking about adding some sh script test, do you think there is some tool we could benefit from?

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
action.yml Outdated Show resolved Hide resolved
@teresaromero teresaromero merged commit 993f33e into main Sep 13, 2023
maroshii pushed a commit that referenced this pull request Mar 12, 2024
* refactor: remove deprecated global argument

Signed-off-by: Teresa Romero <[email protected]>

* refactor: fix file argument

Signed-off-by: Teresa Romero <[email protected]>

* refactor: include tag param into Readme and entrypoint

Signed-off-by: Teresa Romero <[email protected]>

* refactor: include path into command

Signed-off-by: Teresa Romero <[email protected]>

* docs: README update

Signed-off-by: Teresa Romero <[email protected]>

* refactor: add buildargs support

Signed-off-by: Teresa Romero <[email protected]>

* docs: README update based on #17

Signed-off-by: Teresa Romero <[email protected]>

* refactor: fix build-args support

Signed-off-by: Teresa Romero <[email protected]>

* refactor: support other flags for build command

Signed-off-by: Teresa Romero <[email protected]>

* Apply suggestions from code review

Co-authored-by: Andrea Falzetti <[email protected]>

---------

Signed-off-by: Teresa Romero <[email protected]>
Co-authored-by: Andrea Falzetti <[email protected]>
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.

4 participants