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

tenv failing to install #308

Closed
ego93 opened this issue Sep 30, 2024 · 11 comments · Fixed by #309
Closed

tenv failing to install #308

ego93 opened this issue Sep 30, 2024 · 11 comments · Fixed by #309

Comments

@ego93
Copy link

ego93 commented Sep 30, 2024

Describe the bug

I am running v11.4.3 and running into an issue with the install step of tenv v3.2.2

 dpkg-deb: error: 'tenv_v3.2.2_amd64.deb' is not a Debian format archive
dpkg: error processing archive tenv_v3.2.2_amd64.deb (--install):
 dpkg-deb --control subprocess returned error exit status 2
Errors were encountered while processing:
 tenv_v3.2.2_amd64.deb

I've also noticed that the install command of tenv is hardcoded to use amd64, which means it will not work for arm or other types or system architecture.

curl --remote-name --no-progress-meter --location "https://github.com/tofuutils/tenv/releases/latest/download/tenv_${TENV_VERSION}_amd64.deb"

sudo dpkg --install "tenv_${TENV_VERSION}_amd64.deb"

Logs


Run devsectop/tf-via-pr@v11
  with:
    arg_chdir: ***
    arg_recursive: false
    tenv_version: v3.2.2
    arg_lock: false
    arg_get: true
    fmt_enable: false
    tf_version: 1.9.6
    cache_plugins: false
    comment_pr: true
    label_pr: true
    plan_parity: false
    tf_tool: terraform
    update_comment: false
    validate_enable: false
    arg_command: plan
    arg_diff: true
    arg_list: false
    arg_out: tfplan
    arg_write: false
  env:
    TERRAFORM_DIR: ***
    PATH_REGION: eu-west-1
    AWS_DEFAULT_REGION: eu-west-1
    AWS_REGION: eu-west-1
    AWS_ACCESS_KEY_ID: ***
    AWS_SECRET_ACCESS_KEY: ***
    AWS_SESSION_TOKEN: ***
    TF_VAR_assume_role: terraform-actions

To Reproduce

      - name: Provision TF
        uses: devsectop/tf-via-pr@v11
        with:
          tenv_version: v3.2.2

Expected behavior

I would expect tenv to install without error

Screenshots

Additional context

tenv_version is not tested in your test workflow

@ego93
Copy link
Author

ego93 commented Sep 30, 2024

Come to think about it the curl command to download a specified version is not right, should it not be

curl --remote-name --no-progress-meter --location "https://github.com/tofuutils/tenv/releases/download/${TENV_VERSION}/tenv_${TENV_VERSION}_amd64.deb"

@ego93
Copy link
Author

ego93 commented Sep 30, 2024

from my checks this only seems to be an issue when defining the tenv version that is not latest.
When you define the newest version (v3.2.3) or not define any version (using latest) its not an issue.

@rdhar
Copy link
Member

rdhar commented Sep 30, 2024

Hi @ego93, really appreciate the comprehensive issue and your follow-up steps as well — stellar report.

from my checks this only seems to be an issue when defining the tenv version that is not latest.
When you define the newest version (v3.2.3) or not define any version (using latest) its not an issue.

Might be worth noting that tenv_version parameter was introduced just a few weeks ago due to reports of breaking changes brought about by tenv v3.2.0 (PR tofuutils/tenv#247).

Come to think about it the curl command to download a specified version is not right, should it not be...

I suspect you're spot-on: great catch, thank you for identifying!

I've also noticed that the install command of tenv is hardcoded to use amd64, which means it will not work for arm or other types or system architecture.

Another great spot. I think I took GitHub's hosted runner for granted and didn't consider alternative architectures.

Though, this issue on the whole has me wondering: should I really be packaging tenv in as part of this Action in the first place? Surely end-users should be able to bring their own Terraform/Tofu binary manager, and TF-via-PR should only focus on the provisioning part, not binary installation?

For existing users reliant on tenv, we can add another example workflow with the relevant steps needed to separately install tenv before running this Action.

@ego93
Copy link
Author

ego93 commented Sep 30, 2024

@rdhar happy to help where I can for the community.

I find it handy that tenv is included in this action, but I also see your point on why it shouldn't be.

It's simple enough to install ourselves however for the end-users without knowledge of writing workflows, tenv doesn't seem to have an action of there own example: setup-tenv
I guess as long as we have good examples on how to do this it shouldn't really be an issue. And you can lighten the load on this action to only do what it needs to do.

Just make sure if you decide to remove tenv change the major version, as this would be a breaking change for some.

@rdhar
Copy link
Member

rdhar commented Sep 30, 2024

Great recommendations.

Admittedly, I'm still keen to continue packaging tenv just to make it easier for newer users to get going without the added terraform/tofu_wrappper requirement from setup-terraform/tofu actions.

However, it's the architecture bit that's really thrown me off.

Sure, it's easy enough for me to add yet another input parameter to select the architecture (or extend tenv_version parameter to accept the architecture suffix as well). But at that point, I expect the user knows enough to copy/paste tenv installation steps directly from their readme.

And, thinking it through, wouldn't new users gravitate towards setup-terraform/tofu actions anyway? Only those looking for tenv-specific benefits would seek it out in the first place (for example, I reached for it when I needed to test TF-via-PR against both terraform and tofu simultaneously as part of the same workflow.

I reckon next steps should be:

  • Release v11 patch version to fix-up the curl command that you spotted.
  • (later on) Release v12 without in-built tenv but comprehensive example to document its usage with TF-via-PR.

Sound like a plan?

@ego93
Copy link
Author

ego93 commented Sep 30, 2024

Sounds like a plan to me.

Re the architecture, from other actions I have used it can go two ways:

  1. We can automatically detect the system architecture and set it accordingly,
    GitHub has the RUNNER_ARCH variable that could help with this.
  2. Let the user define the architecture with the default to amd64.

As an ideal solution you could combine the both with it auto detecting the arch but let the user manually override for edge cases.

I just wanted to make it clear that I'm not affected by the architecture issue, it's just something I noticed when review your code.

@rdhar
Copy link
Member

rdhar commented Oct 1, 2024

As an aside, I have to ask: what use-case made you opt for tenv over existing actions like setup-terraform/opentofu, @ego93?

Edit: this issue should be resolved as of v11.4.4 release. Thanks once again for raising!

@ego93
Copy link
Author

ego93 commented Oct 1, 2024

As an aside, I have to ask: what use-case made you opt for tenv over existing actions like setup-terraform/opentofu

@rdhar in my use case I'm in the process of migrating from Terraform to OpenToFu, with tenv it makes it simple for me to switch tool based on directory. However I can use the setup-terraform/opentofu with a if statement.

@rdhar
Copy link
Member

rdhar commented Oct 1, 2024

Super-interesting. Pretty much aligned with my use-case as well in terms of needing to context-switch between terraform and tofu dynamically.

Since there isn't really a sizable user-base of TF-via-PR, it's tricky get meaningful data from a poll in order to help decide on continued inclusion of tenv as part of this action.

I'll ask around to try and scrape together some feedback. Might be worth asking tenv directly for their thoughts as well.

@ego93
Copy link
Author

ego93 commented Oct 1, 2024

Thank you by the way for getting the fix out, and for creating this action. It's been a lifesaver in my move to GitHub Actions.

@rdhar
Copy link
Member

rdhar commented Oct 1, 2024

Ah, no worries — and thank you for the sponsorship! Now I've gotta figure out how to pass it on to the tenv folks.

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 a pull request may close this issue.

2 participants