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.

Sign up for GitHub

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

Jump to bottom

Move automation default vars into seperate scenario files #375

Merged
merged 1 commit into from
Aug 30, 2024
Merged

Move automation default vars into seperate scenario files #375

merged 1 commit into from
Aug 30, 2024

Conversation

raukadah
Copy link
Contributor

#374 adds the trigger job which will run different Baremetal VA jobs downstream on different architecture file changes.

Currently automation/vars/default.yaml contains different multiple scenarios, stored in a single file. Trigger job may run unwanted jobs downstream without testing the proper architecture changes.

By moving automations vars into different scenario files allow us to run selective trigger job and test the proper prs.

@openshift-ci openshift-ci bot requested review from abays and cjeanner August 27, 2024 13:43
raukadah added a commit to openstack-k8s-operators/ci-framework that referenced this pull request Aug 27, 2024
@raukadah
Use cifmw_arch_automation_file instead of default automation file
7bad0da 
openstack-k8s-operators/architecture#375 splits
the default vars into different scenario files. We need to fix the
reproducer to adapt this change.

This pull-request adds cifmw_arch_automation_file var to include
different scenario files.

Depends-On: openstack-k8s-operators/architecture#375

Signed-off-by: Chandan Kumar (raukadah) <raukadah@gmail.com>
This was referenced Aug 27, 2024
Merged
Merged
raukadah added a commit to openstack-k8s-operators/ci-framework that referenced this pull request Aug 28, 2024
@raukadah
Use cifmw_arch_automation_file instead of default automation file
a1d4dd5 
openstack-k8s-operators/architecture#375 splits
the default vars into different scenario files. We need to fix the
reproducer to adapt this change.

This pull-request adds cifmw_arch_automation_file var to include
different scenario files.

Depends-On: openstack-k8s-operators/architecture#375

Signed-off-by: Chandan Kumar (raukadah) <raukadah@gmail.com>
@raukadah raukadah force-pushed the split_default_automation branch 3 times, most recently from 3bc9ccd to 8380941 Compare August 29, 2024 07:55
@abays
Copy link
Contributor

abays commented Aug 29, 2024

This looks fine to me. Do we need to merge a change to CIFMW in parallel, @cjeanner ?

@cjeanner
Copy link
Contributor

@abays we indeed need a change on our side. @raukadah has proposed one already (openstack-k8s-operators/ci-framework#2267), but we can make it better by ensuring the file name matches the scenario name.

BTW, we might want to get a new linter here, in architecture repo, to ensure files only have ONE scenario, and that file name matches the scenario. WDYT?

@abays
Copy link
Contributor

abays commented Aug 29, 2024

@cjeanner A new linter as you suggest sounds like a solid idea 👍

@cjeanner
Copy link
Contributor

@abays I think we can edit the YAML schema check to get only one entry in vas. Then, we should be able to add a new thing regarding the filename vs scenario name. I can have a look once this PR is in.

raukadah added a commit to openstack-k8s-operators/ci-framework that referenced this pull request Aug 29, 2024
@raukadah
Use cifmw_architecture_scenario to set proper automation file
9db8391 
openstack-k8s-operators/architecture#375 splits
the default vars into different scenario files. We need to fix the
reproducer to adapt this change.

This pull-request uses cifmw_architecture_scenario to set proper automation file.

Depends-On: openstack-k8s-operators/architecture#375

Signed-off-by: Chandan Kumar (raukadah) <raukadah@gmail.com>
raukadah added a commit to openstack-k8s-operators/ci-framework that referenced this pull request Aug 29, 2024
@raukadah
Use cifmw_architecture_scenario to set proper automation file
c3a0c54 
openstack-k8s-operators/architecture#375 splits
the default vars into different scenario files. We need to fix the
reproducer to adapt this change.

This pull-request uses cifmw_architecture_scenario to set proper automation file.

Depends-On: openstack-k8s-operators/architecture#375

Signed-off-by: Chandan Kumar (raukadah) <raukadah@gmail.com>
@tosky
Copy link
Contributor

tosky commented Aug 29, 2024

Shouldn't this change be done in multiple steps?

  1. add the split files
  2. migrated the users
  3. remove the default file

@cjeanner
Copy link
Contributor

I have a linter that seems to do what we want.
For instance:

AssertionError: !! bgp.yaml does not match bgp_dt01.yaml

and yamele is also unhappy:

yamale -s .ci/automation-schema.yaml automation/vars
Finding yaml files...
Found 15 yaml files.
Validating...
Validation failed!
Error validating data '/home/cjeanner/work/github.com/openstack-k8s-operators/architecture/automation/vars/bgp.yaml' with schema '.ci/automation-schema.yaml'

So we're getting to a nice place.

cjeanner added a commit that referenced this pull request Aug 29, 2024
@cjeanner
Ensure we have one scenario per automation file
7f411ed 
This is in addition to a coming PR, #375. This change here is allowing
to ensure we follow those practices:
- only one scenario per file
- filename matches "scenario_name.yaml" pattern
@cjeanner cjeanner mentioned this pull request Aug 29, 2024
Merged
@cjeanner
Copy link
Contributor

#379 FYI (it's independent from that PR)

@abays
Copy link
Contributor

abays commented Aug 29, 2024

Shouldn't this change be done in multiple steps?

1. add the split files

2. migrated the users

3. remove the default file

That might be the smoother way to transition this. What do you think @raukadah and @cjeanner ?

@raukadah
Move automation default vars into seperate scenario files
216f1cc 
openstack-k8s-operators#374 adds
the trigger job which will run different Baremetal VA jobs downstream on
different architecture file changes.

Currently automation/vars/default.yaml contains different multiple
scenarios, stored in a single file. Trigger job may run unwanted jobs
downstream without testing the proper architecture changes.

By moving automations vars into different scenario files allow us
to run selective trigger job and test the proper prs.

Let's keep the defaults.yaml file till we finish the migration.

Signed-off-by: Chandan Kumar (raukadah) <raukadah@gmail.com>
cjeanner added a commit that referenced this pull request Aug 30, 2024
@cjeanner
Ensure we have one scenario per automation file
9a50bdf 
This is in addition to a coming PR, #375. This change here is allowing
to ensure we follow those practices:
- only one scenario per file
- filename matches "scenario_name.yaml" pattern
raukadah added a commit to openstack-k8s-operators/ci-framework that referenced this pull request Aug 30, 2024
@raukadah
Use cifmw_architecture_scenario to set proper automation file
3273071 
openstack-k8s-operators/architecture#375 splits
the default vars into different scenario files. We need to fix the
reproducer to adapt this change.

This pull-request uses cifmw_architecture_scenario to set proper automation file.
It drops cifmw_arch_automation_file params from scenario files.

Depends-On: openstack-k8s-operators/architecture#375

Signed-off-by: Chandan Kumar (raukadah) <raukadah@gmail.com>
@cjeanner
Copy link
Contributor

@abays @raukadah proposal sounds good indeed, allowing to not have to hurry-merge the ci-framework side.
I think the current code state is doing that now, correct?

So we can go with it. My linter is ready, once we're good with the migration, we'll be able to merge it and make things consistent. It can also be used to test the cleaning PR, btw :).

@raukadah
Copy link
Contributor Author

@abays @raukadah proposal sounds good indeed, allowing to not have to hurry-merge the ci-framework side. I think the current code state is doing that now, correct?

So we can go with it. My linter is ready, once we're good with the migration, we'll be able to merge it and make things consistent. It can also be used to test the cleaning PR, btw :).

sounds good to me.

cjeanner added a commit that referenced this pull request Sep 3, 2024
@cjeanner
Ensure we have one scenario per automation file
b4c9b75 
This is in addition to a coming PR, #375. This change here is allowing
to ensure we follow those practices:
- only one scenario per file
- filename matches "scenario_name.yaml" pattern
openshift-merge-bot bot pushed a commit to openstack-k8s-operators/ci-framework that referenced this pull request Sep 3, 2024
@raukadah @openshift-merge-bot
Use cifmw_architecture_scenario to set proper automation file
7557d20 
openstack-k8s-operators/architecture#375 splits
the default vars into different scenario files. We need to fix the
reproducer to adapt this change.

This pull-request uses cifmw_architecture_scenario to set proper automation file.
It drops cifmw_arch_automation_file params from scenario files.

Depends-On: openstack-k8s-operators/architecture#375

Signed-off-by: Chandan Kumar (raukadah) <raukadah@gmail.com>
cjeanner pushed a commit to openstack-k8s-operators/ci-framework that referenced this pull request Sep 3, 2024
@raukadah @cjeanner
Use cifmw_architecture_scenario to set proper automation file
e80b22d 
openstack-k8s-operators/architecture#375 splits
the default vars into different scenario files. We need to fix the
reproducer to adapt this change.

This pull-request uses cifmw_architecture_scenario to set proper automation file.
It drops cifmw_arch_automation_file params from scenario files.

Depends-On: openstack-k8s-operators/architecture#375

Signed-off-by: Chandan Kumar (raukadah) <raukadah@gmail.com>
@cjeanner cjeanner mentioned this pull request Sep 3, 2024
Merged
openshift-merge-bot bot pushed a commit to openstack-k8s-operators/ci-framework that referenced this pull request Sep 3, 2024
@raukadah @openshift-merge-bot
Use cifmw_architecture_scenario to set proper automation file
07a6146 
openstack-k8s-operators/architecture#375 splits
the default vars into different scenario files. We need to fix the
reproducer to adapt this change.

This pull-request uses cifmw_architecture_scenario to set proper automation file.
It drops cifmw_arch_automation_file params from scenario files.

Depends-On: openstack-k8s-operators/architecture#375

Signed-off-by: Chandan Kumar (raukadah) <raukadah@gmail.com>
softwarefactory-project-zuul bot added a commit that referenced this pull request Sep 4, 2024
@softwarefactory-project-zuul
Merge pull request #384 from raukadah/split_bgp
05942d3 
Move bgp_dt01 into its own automation file

#375 splits the default vars into different scenario files. We need to follow similar pattern for bgp_dt01 also.
Depends-On: openstack-k8s-operators/ci-framework#2267

Reviewed-by: Marios Andreou
cjeanner pushed a commit to raukadah/architecture that referenced this pull request Sep 4, 2024
@raukadah @cjeanner
Drop default.yaml file
d52413e 
openstack-k8s-operators#375 splits the default vars into different scenario files.
openstack-k8s-operators/ci-framework#2267
suggests to use Use cifmw_architecture_scenario to set proper automation
file.

So we are no longer needing default.yaml file.

Depends-On: openstack-k8s-operators/ci-framework#2267

Signed-off-by: Chandan Kumar (raukadah) <raukadah@gmail.com>
cjeanner pushed a commit that referenced this pull request Sep 4, 2024
@raukadah @cjeanner
Drop default.yaml file
e14dcdb 
#375 splits the default vars into different scenario files.
openstack-k8s-operators/ci-framework#2267
suggests to use Use cifmw_architecture_scenario to set proper automation
file.

So we are no longer needing default.yaml file.

Depends-On: openstack-k8s-operators/ci-framework#2267

Signed-off-by: Chandan Kumar (raukadah) <raukadah@gmail.com>
cjeanner added a commit that referenced this pull request Sep 4, 2024
@cjeanner
Ensure we have one scenario per automation file
79de818 
This is in addition to a coming PR, #375. This change here is allowing
to ensure we follow those practices:
- only one scenario per file
- filename matches "scenario_name.yaml" pattern
softwarefactory-project-zuul bot added a commit that referenced this pull request Sep 4, 2024
@softwarefactory-project-zuul
Merge pull request #383 from raukadah/drop_defaults
2444537 
Drop default.yaml file

#375 splits the default vars into different scenario files. openstack-k8s-operators/ci-framework#2267 suggests to use Use cifmw_architecture_scenario to set proper automation file.
So we are no longer needing default.yaml file.
Depends-On: openstack-k8s-operators/ci-framework#2292

Reviewed-by: Cédric Jeanneret
cjeanner added a commit that referenced this pull request Sep 4, 2024
@cjeanner
Ensure we have one scenario per automation file
ab9c91e 
This is in addition to a coming PR, #375. This change here is allowing
to ensure we follow those practices:
- only one scenario per file
- filename matches "scenario_name.yaml" pattern
eduolivares pushed a commit to eduolivares/architecture that referenced this pull request Sep 9, 2024
@raukadah @eduolivares
Move bgp_dt01 into its own automation file
2102f44 
openstack-k8s-operators#375 splits the default vars into different scenario files.
We need to follow similar pattern for bgp_dt01 also.

Depends-On: openstack-k8s-operators/ci-framework#2267

Signed-off-by: Chandan Kumar (raukadah) <raukadah@gmail.com>
(cherry picked from commit 116ab90)
@ciecierski
Copy link
Contributor

When are you planing to land this change in 18.0.0-proposed ?

@fultonj
Copy link
Contributor

fultonj commented Sep 9, 2024

@raukadah do you want to cherry pick this to 18.0.0-proposed as described here?

https://github.com/openstack-k8s-operators/architecture/blob/main/docs/contributing/cherry-picking.md

Same for these two?

#383
#379

@raukadah
Copy link
Contributor Author

@ciecierski @fultonj Hello, Since we merged #383 and #379. we saw breakage in downstream jobs due to mixing of upstream and downstream content. We fixed that.
I think it is safe to backport it. But I need acks from @cjeanner @eduolivares @eshulman2 before doing that to avoid breakage.

@raukadah
Copy link
Contributor Author

raukadah commented Sep 10, 2024 •
edited
Loading

After discussing with @cjeanner , we need to cherry-pick following prs

raukadah added a commit to raukadah/architecture that referenced this pull request Sep 10, 2024
@raukadah
Backport default.yaml split patches
23a4e5d 
It backports following patches:
openstack-k8s-operators#375 -  Move automation default vars into seperate scenario files
openstack-k8s-operators#383 - Drop default.yaml file
openstack-k8s-operators#379 - Ensure we have one scenario per automation file
openstack-k8s-operators#384 - Move bgp_dt01 into its own automation file

Signed-off-by: Chandan Kumar (raukadah) <raukadah@gmail.com>
@raukadah raukadah mentioned this pull request Sep 10, 2024
Merged
raukadah added a commit to openstack-k8s-operators/ci-framework that referenced this pull request Sep 10, 2024
@raukadah
Use cifmw_architecture_scenario to set proper automation file
445a412 
openstack-k8s-operators/architecture#375 splits
the default vars into different scenario files. We need to fix the
reproducer to adapt this change.

This pull-request uses cifmw_architecture_scenario to set proper automation file.
It drops cifmw_arch_automation_file params from scenario files.

Depends-On: openstack-k8s-operators/architecture#375

Note: it excludes zuul.d/architecture-jobs.yaml changes.

Cherry-picked from 07a6146#diff-693e3f6ce03f7b6994b1195bcced3d58c89f1c286667356f827c3aecd4518ef6

Signed-off-by: Chandan Kumar (raukadah) <raukadah@gmail.com>
@raukadah raukadah mentioned this pull request Sep 10, 2024
Merged
@raukadah
Copy link
Contributor Author

Cherry-pick patches

Feel free to test it before merging.

raukadah added a commit to raukadah/architecture that referenced this pull request Sep 11, 2024
@raukadah
Backport default.yaml split patches
613f451 
It backports following patches:
openstack-k8s-operators#375 -  Move automation default vars into seperate scenario files
openstack-k8s-operators#383 - Drop default.yaml file
openstack-k8s-operators#379 - Ensure we have one scenario per automation file
openstack-k8s-operators#384 - Move bgp_dt01 into its own automation file
openstack-k8s-operators#373 - NFV-HCI minor fixes

Signed-off-by: Chandan Kumar (raukadah) <raukadah@gmail.com>
openshift-merge-bot bot pushed a commit to openstack-k8s-operators/ci-framework that referenced this pull request Sep 11, 2024
@raukadah @openshift-merge-bot
Use cifmw_architecture_scenario to set proper automation file
8cec1bb 
openstack-k8s-operators/architecture#375 splits
the default vars into different scenario files. We need to fix the
reproducer to adapt this change.

This pull-request uses cifmw_architecture_scenario to set proper automation file.
It drops cifmw_arch_automation_file params from scenario files.

Depends-On: openstack-k8s-operators/architecture#375

Note: it excludes zuul.d/architecture-jobs.yaml changes.

Cherry-picked from 07a6146#diff-693e3f6ce03f7b6994b1195bcced3d58c89f1c286667356f827c3aecd4518ef6

Signed-off-by: Chandan Kumar (raukadah) <raukadah@gmail.com>
softwarefactory-project-zuul bot added a commit that referenced this pull request Sep 11, 2024
@softwarefactory-project-zuul
Merge pull request #394 from raukadah/sync_patches
ab2e02a 
Backport default.yaml split patches

It backports following patches:

#375
#383
#379
#384
#373

Reviewed-by: Ella Shulman <ellashulman1@gmail.com>
Reviewed-by: Cédric Jeanneret
raukadah added a commit to openstack-k8s-operators/ci-framework that referenced this pull request Sep 12, 2024
@raukadah
Use cifmw_architecture_scenario file in set va scenario
c138d97 
openstack-k8s-operators/architecture#383 drops
defaults.yaml scenario file in favor of
openstack-k8s-operators/architecture#375.

#2267
suggests to use  Use cifmw_architecture_scenario to set proper
automation file.

Whereever default.yaml is used, the job broke. This pr fixes the
same using cifmw_architecture_scenario file.

Signed-off-by: Chandan Kumar (raukadah) <raukadah@gmail.com>
@raukadah raukadah mentioned this pull request Sep 12, 2024
Merged
raukadah added a commit to openstack-k8s-operators/ci-framework that referenced this pull request Sep 12, 2024
@raukadah
Use cifmw_architecture_scenario file in set va scenario
347ce1a 
openstack-k8s-operators/architecture#383 drops
defaults.yaml scenario file in favor of
openstack-k8s-operators/architecture#375.

#2267
suggests to use  Use cifmw_architecture_scenario to set proper
automation file.

Whereever default.yaml is used, the job broke. This pr fixes the
same using cifmw_architecture_scenario file.

Signed-off-by: Chandan Kumar (raukadah) <raukadah@gmail.com>
openshift-merge-bot bot pushed a commit to openstack-k8s-operators/ci-framework that referenced this pull request Sep 12, 2024
@raukadah @openshift-merge-bot
Use cifmw_architecture_scenario file in set va scenario
b2559d6 
openstack-k8s-operators/architecture#383 drops
defaults.yaml scenario file in favor of
openstack-k8s-operators/architecture#375.

#2267
suggests to use  Use cifmw_architecture_scenario to set proper
automation file.

Whereever default.yaml is used, the job broke. This pr fixes the
same using cifmw_architecture_scenario file.

Signed-off-by: Chandan Kumar (raukadah) <raukadah@gmail.com>
openshift-cherrypick-robot pushed a commit to openshift-cherrypick-robot/ci-framework that referenced this pull request Sep 12, 2024
@raukadah
Use cifmw_architecture_scenario file in set va scenario
d8cf5b1 
openstack-k8s-operators/architecture#383 drops
defaults.yaml scenario file in favor of
openstack-k8s-operators/architecture#375.

openstack-k8s-operators#2267
suggests to use  Use cifmw_architecture_scenario to set proper
automation file.

Whereever default.yaml is used, the job broke. This pr fixes the
same using cifmw_architecture_scenario file.

Signed-off-by: Chandan Kumar (raukadah) <raukadah@gmail.com>
openshift-merge-bot bot pushed a commit to openstack-k8s-operators/ci-framework that referenced this pull request Sep 12, 2024
@raukadah @openshift-merge-bot
Use cifmw_architecture_scenario file in set va scenario
db4b0d8 
openstack-k8s-operators/architecture#383 drops
defaults.yaml scenario file in favor of
openstack-k8s-operators/architecture#375.

#2267
suggests to use  Use cifmw_architecture_scenario to set proper
automation file.

Whereever default.yaml is used, the job broke. This pr fixes the
same using cifmw_architecture_scenario file.

Signed-off-by: Chandan Kumar (raukadah) <raukadah@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fultonj fultonj fultonj approved these changes

@abays abays Awaiting requested review from abays

@cjeanner cjeanner Awaiting requested review from cjeanner

Assignees

@fultonj fultonj

Labels
approved lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@raukadah @abays @cjeanner @tosky @fultonj @ciecierski