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

support os_distro and version for CI job #1726

Merged
merged 2 commits into from
Mar 15, 2024
Merged

Conversation

Issacwww
Copy link
Member

Issue #, if available:

Description of changes:
Extend the CI job to accept os_distro and version to be frugal

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Testing Done

Tested in my folk Issacwww#3

See this guide for recommended testing for PRs. Some tests may not apply. Completing tests and providing additional validation steps are not required, but it is recommended and may reduce review time and time to merge.

@cartermckinnon
Copy link
Member

cartermckinnon commented Mar 14, 2024

I like the change, but don't love the syntax 😛 . What if we used the + syntax like we do for goal-specific arguments, and handled a workflow "goal" to pass arbitrary arguments to the ci-manual.yaml workflow?

/ci
+workflow matrix_os_distros=al2
+workflow matrix_k8s_versions=1.21,1.22

@Issacwww
Copy link
Member Author

Fair fair, I will update the syntax

@Issacwww
Copy link
Member Author

Issacwww commented Mar 14, 2024

I think this format would be good, as the later arguments would override the earlier if we pass then in multiple lines, here. Change this to a list is not that necessary as build_arguments also take multi args in one line

/ci
+workflow matrix_os_distros=al2,al2023 matrix_k8s_versions=1.21,1.22,1.23

Tested here

@cartermckinnon
Copy link
Member

cartermckinnon commented Mar 14, 2024

Parsing multiple args out of a single +workflow line is a little messy, what about:

/ci
+workflow:os_distros al2,al2023
+workflow:k8s_versions 1.21,1.22

Then we can just check if the goal.startsWith("workflow:") and do some simple splitting to get the arg name and its value?

I think it'll be more common to set only one of these dimensions of the matrix vs. both, so the extra +workflow:... line doesn't seem like much of a downside

edit: lol triggered the CI

Copy link
Contributor

@cartermckinnon roger that! I've dispatched a workflow. 👍

Copy link
Contributor

@cartermckinnon the workflow that you requested has completed. 🎉

AMI variantBuildTest
1.23 / al2cancelled 🚮skipped ⏭️
1.23 / al2023cancelled 🚮skipped ⏭️
1.24 / al2cancelled 🚮skipped ⏭️
1.24 / al2023cancelled 🚮skipped ⏭️
1.25 / al2cancelled 🚮skipped ⏭️
1.25 / al2023cancelled 🚮skipped ⏭️
1.26 / al2cancelled 🚮skipped ⏭️
1.26 / al2023cancelled 🚮skipped ⏭️
1.27 / al2cancelled 🚮skipped ⏭️
1.27 / al2023cancelled 🚮skipped ⏭️
1.28 / al2cancelled 🚮skipped ⏭️
1.28 / al2023cancelled 🚮skipped ⏭️
1.29 / al2cancelled 🚮skipped ⏭️
1.29 / al2023cancelled 🚮skipped ⏭️

@Issacwww
Copy link
Member Author

Updated new revision with multi-line arguments, tested here

@Issacwww Issacwww merged commit 55b73b0 into awslabs:main Mar 15, 2024
10 checks passed
joelddiaz pushed a commit to joelddiaz/amazon-eks-ami that referenced this pull request May 29, 2024
* support os_distro and version for CI job

* Update bot syntax
atmosx pushed a commit to gathertown/amazon-eks-ami that referenced this pull request Jun 18, 2024
* support os_distro and version for CI job

* Update bot syntax
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