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

CI Documentation - added machine list table and label columns #317

Merged
merged 1 commit into from
Dec 18, 2024

Conversation

ryansavino
Copy link
Member

@ryansavino ryansavino commented Jan 15, 2024

The added machine table links the machine's assigned labels to the jobs required labels.
Corrected some mistakes and label assignments.
Formatting.

Fixes: #316

Signed-Off-By: Ryan Savino [email protected]

@wainersm
Copy link
Member

hi @ryansavino , I think that a separated table only to map label -> machine would be better.

@portersrc
Copy link
Member

lgtm; wainer's comment is fine, too, if you think it would help

@ldoktor
Copy link
Contributor

ldoktor commented Jan 24, 2024

Looks good to me as is in the single table (it's not that wide) although I'm having troubles actually finding the pipeline definitions (tried searching for the name as well as the labels). Are they supposed to be already there?

@ryansavino ryansavino force-pushed the ci-doc-amd-nodes branch 2 times, most recently from 8efd6ad to daacda4 Compare February 7, 2024 18:34
@ryansavino
Copy link
Member Author

I've updated this PR with the machine table as well. @wainersm let me know what you think. I can add other system labels if you guys know what they are. @BbolroC @stevenhorsman @fidencio

Question: default CcRuntime is referring to runtime class? Should I add that column in the job table?

@ldoktor the AMD nodes are already labeled.

docs/DEVELOPMENT.md Outdated Show resolved Hide resolved
docs/DEVELOPMENT.md Outdated Show resolved Hide resolved
@fidencio
Copy link
Member

fidencio commented Aug 4, 2024

I can add other system labels if you guys know what they are. @BbolroC @stevenhorsman @fidencio

Please, if possible, do.

image

So, for the TDX machine the label is tdx, for the s390x machine the label is S390X.

@ryansavino ryansavino requested a review from a team as a code owner December 10, 2024 22:01
@ryansavino ryansavino changed the title CI Documentation - Add AMD nodes and labels to jobs table CI Documentation - added machine list table and label columns Dec 10, 2024
@ryansavino
Copy link
Member Author

I can add other system labels if you guys know what they are. @BbolroC @stevenhorsman @fidencio

Please, if possible, do.

image

So, for the TDX machine the label is tdx, for the s390x machine the label is S390X.

Done.

Comment on lines 99 to 100
| az-ubuntu-2004 | virtual | Non-TEE | |
| az-ubuntu-2204 | virtual | Non-TEE | |
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we use these runners any more, but just the github hosted ones?

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed. Thanks.

| --- | --- | --- | --- |
| az-ubuntu-2004 | virtual | Non-TEE | |
| az-ubuntu-2204 | virtual | Non-TEE | |
| s390x-runner-01 | virtual | Non-TEE | S390X |
Copy link
Member

Choose a reason for hiding this comment

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

For the runner s390x-runner-01, s390x and s390x-large are used. There is another one named s390x-runner-02 whose label is only s390x.

Copy link
Member

Choose a reason for hiding this comment

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

So the final status would look like:

| s390x-runner-01 | virtual | Non-TEE | s390x, s390x-large
| s390x-runner-02 | virtual | Non-TEE | s390x

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@ryansavino ryansavino force-pushed the ci-doc-amd-nodes branch 2 times, most recently from c4bcf57 to b62e9d9 Compare December 11, 2024 17:10
Copy link
Member

@stevenhorsman stevenhorsman left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @ryansavino!

|operator enclave-cc e2e tests| Intel SGX (Simulated Mode) | Ubuntu 22.04 |
| Job name | TEE | OS | Required Labels |
| --- | --- | --- | --- |
| operator enclave-cc e2e tests | Intel SGX (Simulated Mode) | Ubuntu 22.04 | ubuntu-22.04 |
Copy link
Member

Choose a reason for hiding this comment

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

I think the Required Labels can be dropped from this line here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

The added machine table links the machine's assigned labels to the jobs required labels.
Corrected some mistakes and label assignments.
Formatting.

Fixes: confidential-containers#316

Signed-Off-By: Ryan Savino <[email protected]>
Copy link
Member

@fidencio fidencio left a comment

Choose a reason for hiding this comment

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

lgtm, thanks @ryansavino!

@fidencio fidencio merged commit 511f62a into confidential-containers:main Dec 18, 2024
17 of 18 checks passed
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.

Add amd node tests and labels to documentation
7 participants