Skip to content

contrib/aws: add non-blocking persistent manual cluster (PMC) test stages - #12743

Open
nmazzilli3 wants to merge 1 commit into
ofiwg:mainfrom
nmazzilli3:pmc_jenkins
Open

nmazzilli3 wants to merge 1 commit into
ofiwg:mainfrom
nmazzilli3:pmc_jenkins

Conversation

@nmazzilli3

Copy link
Copy Markdown
Contributor

contrib/aws: add non-blocking persistent manual cluster (PMC) test stages

Adds PMC test stages that run in parallel with the existing ephemeral PR CI stages. The existing stages are unchanged and still gate the PR; the PMC stages are non-blocking (wrapped in catchError → UNSTABLE, never touch build_ok) so they can A/B the persistent-cluster path against the current create-then-destroy path on every run.

  • New helpers + build_pmc_test_stages(), merged into the existing parallel block.
  • PMC stages attach to pre-created persistent clusters by public IP (from the Jenkins lock→IP SSM mapping) and run tests in-container instead of creating/destroying a cluster per run.
  • Tests the same PR/commit as the ephemeral stages (--test-libfabric-pr $CHANGE_ID, or $GIT_COMMIT post-merge).
  • Uses separate lock labels so the two paths don't contend for capacity.

Validated on the internal persistent-cluster POC across all 9 clusters (c5n, c7g-ub22, c7g-ub24, hpc7g, g4dn, hpc8a, hpc6a, c8gn, c7i).

@nmazzilli3
nmazzilli3 force-pushed the pmc_jenkins branch 4 times, most recently from e4ce152 to f3fa01e Compare August 31, 2026 17:35
Comment thread contrib/aws/Jenkinsfile Outdated
@nmazzilli3
nmazzilli3 force-pushed the pmc_jenkins branch 4 times, most recently from 3f73c1e to 85c7497 Compare September 2, 2026 04:12
@nmazzilli3

Copy link
Copy Markdown
Contributor Author

bot:aws:retest

1 similar comment
@nmazzilli3

Copy link
Copy Markdown
Contributor Author

bot:aws:retest

@nmazzilli3
nmazzilli3 force-pushed the pmc_jenkins branch 2 times, most recently from 14c137b to 42aa1f7 Compare September 8, 2026 22:28
Comment thread contrib/aws/Jenkinsfile
Comment thread contrib/aws/Jenkinsfile Outdated
Comment thread contrib/aws/Jenkinsfile Outdated
Comment thread contrib/aws/Jenkinsfile
stages["pmc_2_hpc6a_rhel8_efa"] = get_test_stage_with_lock_persistent("pmc_2_hpc6a_rhel8_efa", "hpc6a", 2, efa_addl_args)
stages["pmc_2_c8gn_alinux2023_efa"] = get_test_stage_with_lock_persistent("pmc_2_c8gn_alinux2023_efa", "c8gn", 2, efa_addl_args)
stages["pmc_2_c7i_alinux2023_tcp"] = get_test_stage_with_lock_persistent("pmc_2_c7i_alinux2023_tcp", "c7i", 2, tcp_addl_args)
stages["pmc_1_g4dn_alinux2023_shm"] = get_test_stage_with_lock_persistent("pmc_1_g4dn_alinux2023_shm", "g4dn", 1, shm_addl_args)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

pmc_1_g4dn_alinux2023_shm switched OS test coverage from ub24 to al2023 silently

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes with an active backlog ticket and decision needed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can replace with a large TODO dedicated to this.

Comment thread contrib/aws/Jenkinsfile
}
}

def build_pmc_test_stages() {

@a-szegel a-szegel Sep 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't want this second function, I want to do everything in 1 function and use the same vars as before (to the extent that we can... for test matrix at least) ... this way, when we make changes to our test matrix, we don't have to do it in two places and they don't drift/get dropped.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

im not following, could you be more specific with what you are looking to gain from this and why having a function is bad here.

@a-szegel a-szegel Sep 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There is a chance both build_pmc_test_stages() and build_pr_test_stages() might exist in parallel for a long time. In that time frame, there is a chance someone wants to update the Jenkinsfile to control what tests run. Rather than copy/paste the same test logic in 2 places, the request was to either use common variables to define the test matrix (make updates in 1 place, potentially by moving both to one function).

All of these are copied/pasted twice (not 100% due to ${generic_pf} being different... but you get the point)

    def efa_addl_args = "${timeout} ${generic_pf} --test-libfabric-provider efa --test-list test_pr_ci_fabtests test_run_efa_unit_tests ${job_args}${persistent_manual_cluster_addl_args}${container_addl_args}"
    def efa_hpc_addl_args = "${timeout} ${generic_pf} --test-libfabric-provider efa --test-list test_pr_ci_fabtests test_pr_ci_imb test_run_efa_unit_tests ${job_args}${persistent_manual_cluster_addl_args}${container_addl_args}"
    def efa_hpc_dso_addl_args = "${timeout} ${generic_pf} --test-libfabric-provider efa --test-list test_pr_ci_fabtests test_pr_ci_imb test_run_efa_unit_tests test_pr_ci_dso ${job_args}${persistent_manual_cluster_addl_args}${container_addl_args}"
    def tcp_addl_args = "${timeout} ${generic_pf} --test-libfabric-provider tcp --enable-efa false --test-list test_pr_ci_fabtests test_run_efa_unit_tests ${job_args}${persistent_manual_cluster_addl_args}${container_addl_args}"
    def shm_addl_args = "${timeout} ${generic_pf} --test-libfabric-provider shm --enable-efa false --test-list test_pr_ci_fabtests test_run_efa_unit_tests ${job_args}${persistent_manual_cluster_addl_args}${container_addl_args}"

Comment thread contrib/aws/Jenkinsfile
}

def build_pmc_test_stages() {
def stages = [:]

@a-szegel a-szegel Sep 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing/Added Test Stages:

  1. Missing 1_c5n_alinux2023_neuron -> I would like you to add a trn2 stage b/c we have already have capacity and one of the major goals of this effort was to restore trn2 testing.
  2. Missing 2_c7gn_ubuntu2204_efa
  3. Missing 2_c7i_ubuntu2204_sockets
  4. Missing 1_g4dn_alinux2023_sm2
  5. Missing 1_g4dn_ubuntu2404_shm_disable-cma
  6. New pmc_2_c7g_ubuntu2204_efa (not in old matrix)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

  1. trn2 was not part of the initial migration plan. I will send the doc offline
  2. c7gn for ub 22 and 24 seems to be there
  3. We will need to add capacity to fit this test in as the original count was only 6
  4. yes there is open bug for this, we will need to add the single gpu g4dn or set visible cuda devices to 0.
  5. All single node instance tests need further capacity / requirements finalization
  6. isnt this what you said was missing in 2

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  1. The entire point of this project is to add trn2. It is ok if it doesn't come in with the first round, but the project cannot be considered complete without it.
  2. pmc_2_c7g_ubuntu2204_efa is not equal to c7gn. It doesn't look like there is c7gn. We had both c7g and c7gn and that is important for each generation of NIC

3-5. The goal is for the test matrix not to change. Any change needs a large TODO comment before this can get merged.

  1. c7g is not equal to c7gn, they have different hardware.

Comment thread contrib/aws/Jenkinsfile
def shm_addl_args = "${timeout} ${generic_pf} --test-libfabric-provider shm --enable-efa false --test-list test_pr_ci_fabtests test_run_efa_unit_tests ${job_args}${persistent_manual_cluster_addl_args}${container_addl_args}"

stages["pmc_2_c5n_ubuntu2604_efa"] = get_test_stage_with_lock_persistent("pmc_2_c5n_ubuntu2604_efa", "c5n", 2, efa_addl_args)
stages["pmc_2_c7g_ubuntu2204_efa"] = get_test_stage_with_lock_persistent("pmc_2_c7g_ubuntu2204_efa", "c7g-ub22", 2, efa_addl_args)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This didn't exist in previous test matrix.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

c7g is not c7gn

Comment thread contrib/aws/Jenkinsfile
def persistent_manual_cluster_addl_args = " --keep-cluster --skip-fixture-setup --skip-health-checks --use-existing-installer --enable-placement-group false --lean-cluster-setup --use-prebuilt-ami-with-efa-installer true --cleanup-pf-directory true --libfabric-prci true --enable-live-log true"
def container_addl_args = " --test-in-containers-on-ec2"

def pr_selector = is_post_merge() ? "--test-type commit --test-libfabric-commit ${env.GIT_COMMIT}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Post Merge is only supposed to be supported on EFA, this silently changes it to be supported everywhere.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ack, we can remove this.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think this is fixed yet.

…ages

Signed-off-by: Nick Mazzilli <nmazzill@amazon.com>
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.

2 participants