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

8.15: fix smoke tests #14233

Closed
wants to merge 1 commit into from
Closed

8.15: fix smoke tests #14233

wants to merge 1 commit into from

Conversation

endorama
Copy link
Member

@endorama endorama commented Oct 2, 2024

Motivation/summary

tags module was using the build input var, but it was not defined.
This was producing failures like:

Error: Reference to undeclared input variable

  on ../../infra/terraform/modules/tags/output.tf line 7, in locals:
   7:     "build" : var.build

An input variable with the name "build" has not been declared. This variable
can be declared with a variable "build" {} block.

Checklist

How to test these changes

Manually trigger Smoke Test workflow on this branch.

https://github.com/elastic/apm-server/actions/workflows/smoke-tests-schedule.yml

Related issues

tags module was using the build input var, but it was not defined.
This was producing failures like:
Error: Reference to undeclared input variable

  on ../../infra/terraform/modules/tags/output.tf line 7, in locals:
   7:     "build" : var.build

An input variable with the name "build" has not been declared. This variable
can be declared with a variable "build" {} block.
@endorama endorama requested a review from a team as a code owner October 2, 2024 08:44
@endorama
Copy link
Member Author

endorama commented Oct 2, 2024

@marclop
Copy link
Contributor

marclop commented Oct 2, 2024

I think we need to merge this to main, and backport to 8.x, not open against 8.15.

@endorama
Copy link
Member Author

endorama commented Oct 2, 2024

@marclop On main this is correct (see here). The original PR wasn't backported, which is the source for these failures #13841

@kruskall
Copy link
Member

kruskall commented Oct 2, 2024

shouldn't it also reference the var in https://github.com/v1v/apm-server/blob/3bc23c4eb4931033c41ff64238c9c8ae2924da7a/testing/infra/terraform/modules/tags/output.tf#L7 ?

fwiw you can backport a PR with @mergifyio backport <branch>

@endorama
Copy link
Member Author

endorama commented Oct 2, 2024

The previous run failed with the same error message, due to the way I triggered it. I triggered another run the should verify if this change works in https://github.com/elastic/apm-server/actions/runs/11147080061

(but it used the workflow file from main so I'm not sure it tests what I expect). Anyway if it don't pass I think it's safe to merge after @marclop confirm I address his concerns.

@endorama
Copy link
Member Author

endorama commented Oct 2, 2024

@kruskall no because 8.15 already has it. Anyway, as the tests are broken, I opened a backport PR, which makes clearer where this fix come from.

Closing as superseded by #14240 (the only difference is the docs)

@endorama endorama closed this Oct 2, 2024
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.

3 participants