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

Improve e2e science, self-hosted test #796

Merged
merged 2 commits into from
Jan 30, 2024

Conversation

fjammes
Copy link
Contributor

@fjammes fjammes commented Jan 18, 2024

What changes were proposed in this pull request?

Increase memory request to 3GB for raw2science pods

How was this patch tested?

In self-hosted CI

@fjammes fjammes self-assigned this Jan 18, 2024
@fjammes fjammes linked an issue Jan 18, 2024 that may be closed by this pull request
@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

@fjammes fjammes force-pushed the 795-chase-oomkill-error-on-science-self-hosted-ci branch from eff813f to 0e8b6dc Compare January 22, 2024 13:12
@fjammes fjammes changed the title Draft PR to trigger self-hosted science build Improve e2e science, self-hosted test Jan 22, 2024
@fjammes fjammes requested a review from JulienPeloton January 22, 2024 13:22
Copy link
Member

@JulienPeloton JulienPeloton left a comment

Choose a reason for hiding this comment

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

Thanks @fjammes -- it is good to see all the test suite green again :-)
I left minor comments, but it is ready for merging.

count=0
while ! finkctl wait topics --expected "$EXPECTED_TOPICS" --timeout 60s -v1
do
echo "Waiting for topics to be created"
Copy link
Member

Choose a reason for hiding this comment

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

I would even be more specific and echo:

echo "Waiting for $EXPECTED_TOPICS topic(s) to be created"

then
for task in $tasks; do
echo "--------- $task log file ---------"
cat "/tmp/$task.log"
done
kubectl describe pods -l "spark-role in (executor, driver)"
kubectl get pods
echo "ERROR: unable to start fink-broker"
Copy link
Member

Choose a reason for hiding this comment

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

To ease the debug, I would print:

echo "ERROR: unable to start fink-broker in 300 seconds"

e2e/finkconfig_noscience/finkctl.yaml Show resolved Hide resolved
fjammes and others added 2 commits January 30, 2024 11:47
Configure fine-grained resource management
Fix fink startup script error management
Use 3GB memory for raw2science
Configure number of expected topics for e2e test output
@fjammes fjammes force-pushed the 795-chase-oomkill-error-on-science-self-hosted-ci branch from 882631c to f34604a Compare January 30, 2024 10:47
Copy link

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@fjammes fjammes merged commit 5990e34 into master Jan 30, 2024
19 of 22 checks passed
@fjammes fjammes deleted the 795-chase-oomkill-error-on-science-self-hosted-ci branch January 30, 2024 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Chase OOMKill error on science self-hosted CI
2 participants