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

Avoid unnecessary rebuild of the test environment #1919

Merged

Conversation

jmontesi
Copy link
Contributor

@jmontesi jmontesi commented Mar 14, 2024

The logs show that the test environment is built twice before starting running the test cases, which should not be necessary. However, disabling this rebuild of the test environment lead to an error while marshaling the TestConfiguration to JSON format. This PR refactors the way the Preflight test results are processed in order to produce an output of the test environment adequate for JSON marshaling without the need to rebuild it.

The problem came with the direct addition of the struct plibRuntime.Results into our Container struct. This former struct contains an interface that prevents the struct to be properly transformed to JSON. To solve this issue, another struct called PreflightResultsDB that only contains the information needed has been used instead.

The test environment rebuild hid this problem because the Preflight results were cleared in the process, allowing the JSON marshaling to work correctly. The results could be retrieved later (even after being cleared) due to the fact that the old results were stored in the closure that contains the test function (defined in WithCheckFn()).

@dcibot
Copy link
Collaborator

dcibot commented Mar 14, 2024

@ramperher
Copy link
Collaborator

ramperher commented Mar 14, 2024

from change #1919:

The job is failing in the execution of tnf, so I believe this needs to be checked
Right now is not showing the logs, I'll fix that with redhatci/ansible-collection-redhatci-ocp#209

@ramperher
Copy link
Collaborator

/dci-rerun

@dcibot
Copy link
Collaborator

dcibot commented Mar 14, 2024

Copy link
Collaborator

@ramperher ramperher left a comment

Choose a reason for hiding this comment

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

tnf cannot be launched in our deployments, in this job: https://www.distributed-ci.io/jobs/0f0456cb-2996-4615-960f-f237cc09a5cd/files, in the Files section, you can take a look to dci-tnf-execution.log and cnf-certsuite.log to see the errors it is throwing. With this failure, this cannot be merged, else DCI jobs will all fail

The logs show that the test environment is built twice before starting
running the test cases.
@jmontesi jmontesi force-pushed the avoid_rebuilding_test_env branch from 60f29fb to 733492a Compare March 19, 2024 16:22
@dcibot
Copy link
Collaborator

dcibot commented Mar 19, 2024

@dcibot
Copy link
Collaborator

dcibot commented Mar 20, 2024

@jmontesi jmontesi merged commit b1a6e17 into redhat-best-practices-for-k8s:main Mar 20, 2024
19 checks passed
@dcibot
Copy link
Collaborator

dcibot commented Mar 20, 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.

5 participants