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

Propagate errors from preflight in the claim file #1684

Merged

Conversation

edcdavid
Copy link
Member

@edcdavid edcdavid commented Dec 1, 2023

Propagates errors from preflight in the claim file even if preflight fails to run due to connectivity issues, missing docker config, etc...
Also contains temporary workaround for leftover ginkgo code in lifecycle, see #1679

@edcdavid edcdavid changed the title Propagates error from preflight in the claim file even if preflight fails to run due to connectivity issues, missing docker config, etc... Propagates errors from preflight in the claim file Dec 1, 2023
@edcdavid edcdavid changed the title Propagates errors from preflight in the claim file Propagate errors from preflight in the claim file Dec 1, 2023
@edcdavid edcdavid force-pushed the propagate-preflight-skip branch from 2a23824 to a2f1b54 Compare December 4, 2023 18:53
greyerof added a commit to greyerof/certsuite that referenced this pull request Dec 5, 2023
This is a temporary workaround to allow running the app in web server
mode when no kubeconfig file/env var has been provided.

For preflight, the clientsholder needs to be fully and correctly
initialized in order to run the checks, otherwise it won't have any
TestEnvironment to get the containers/operators under test.

For web server mode, there's no need to have a valid clientsholder yet,
as the kubeconfig file is provided via web form, and it can be a
different one on each run. The clientsholder is then created with the
clientsholder.GetNewClientsHolder().

This workaround should probably be removed/refactored when PR redhat-best-practices-for-k8s#1684 is
merged.
…ails to run due to connectivity issues, missing docker config, etc...
@edcdavid edcdavid force-pushed the propagate-preflight-skip branch from a2f1b54 to 82dcfa3 Compare December 5, 2023 21:25
@edcdavid edcdavid force-pushed the propagate-preflight-skip branch from 82dcfa3 to 09926a7 Compare December 5, 2023 21:26
@ramperher
Copy link
Collaborator

/dci-rerun

@ramperher
Copy link
Collaborator

ramperher commented Jan 2, 2024

I just wanted to check that DCI jobs are now correctly scheduled even if they come from ginkgo_removal branch, since we verified it's stable from DCI side. And it's working. So, now, you'll see again DCI jobs in PRs related to ginkgo_removal.

@ramperher
Copy link
Collaborator

DCI job is failing when trying to build the tnf image locally: https://www.distributed-ci.io/jobs/1cd9f0ef-ad33-4eef-80c2-4196b386ff27/jobStates, could be a transient issue. If you need to merge this, it's not a problem, but it's preferrable to have this working if possible.

@edcdavid edcdavid changed the base branch from ginkgo_removal to main January 8, 2024 19:27
@edcdavid
Copy link
Member Author

edcdavid commented Jan 9, 2024

DCI job is failing when trying to build the tnf image locally: https://www.distributed-ci.io/jobs/1cd9f0ef-ad33-4eef-80c2-4196b386ff27/jobStates, could be a transient issue. If you need to merge this, it's not a problem, but it's preferrable to have this working if possible.
This might be due to this PR using depends-on github action. Once the dependency is merged it should be able to pass DCI

Copy link

@Apetree100122 Apetree100122 left a comment

Choose a reason for hiding this comment

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

I will approve please make the changes

Choose a reason for hiding this comment

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

If this is bash or pip3 python please try not to
Align one of the letters under the
Same letters reads /\/

@@ -7,9 +7,9 @@ Depending on the CNF type, not all tests are required to pass to satisfy best pr

## Test cases summary

### Total test cases: 88

Choose a reason for hiding this comment

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

8 8 == 1\
1 0 5 ==4 or 2 == 3 or 7
7 3 ==4 or 6== 10
!== 4 == 105

@@ -35,11 +36,11 @@ Depending on the CNF type, not all tests are required to pass to satisfy best pr
|---|---|
|7|1|

### Non-Telco specific tests only: 41
### Non-Telco specific tests only: 58

Choose a reason for hiding this comment

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

41==3 Or 5 line 38
==5
8 ==3 li ne 39
7 ==(8

  1. ==\9
    4==3
    9 ==6
    ==105 Line 42 and 43
    ====3*20 or 3+2== 5 0
    ===60=
    ===3
    ===11
    +1 ==12 (total test line 27

@@ -573,11 +574,11 @@ Tags|telco,lifecycle

Property|Description
---|---
Unique ID|lifecycle-container-shutdown

Choose a reason for hiding this comment

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

If one - is up In sentence or line then the rest
Follow this is In line 576 different below

Copy link

@Apetree100122 Apetree100122 left a comment

Choose a reason for hiding this comment

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

/close

@dcibot
Copy link
Collaborator

dcibot commented Jan 22, 2024

@dcibot
Copy link
Collaborator

dcibot commented Jan 24, 2024

@edcdavid
Copy link
Member Author

/dci-retest

@edcdavid
Copy link
Member Author

/dci-rerun

@dcibot
Copy link
Collaborator

dcibot commented Jan 24, 2024

@dcibot
Copy link
Collaborator

dcibot commented Jan 26, 2024

@edcdavid edcdavid merged commit 8c87537 into redhat-best-practices-for-k8s:main Jan 26, 2024
19 checks passed
@dcibot
Copy link
Collaborator

dcibot commented Jan 26, 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.

6 participants