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

Introduce "adoption" snippet in automation file #382

Closed
wants to merge 2 commits into from

Conversation

cjeanner
Copy link
Contributor

@cjeanner cjeanner commented Aug 30, 2024

For adoption, there's a need to override the infra exposed by the
scenario.
While this new "adoption" entry creates a stronger link with the
ci-framework, it can also be consumed by any other tool, as long as it
knows how to consume the two "patches".

This is part of a PoC, and will probably lead to discussions regarding
the best way to manage all of the datasets.

Copy link

openshift-ci bot commented Aug 30, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Copy link

openshift-ci bot commented Aug 30, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cjeanner

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

cjeanner added a commit to openstack-k8s-operators/ci-framework that referenced this pull request Aug 30, 2024
This playbook allows to create an infrastructure, mostly related to the
"adoption" use-case.

It allows to consume overrides provided by the automation file shipped
within the architecture repository.

This, alongside
openstack-k8s-operators/architecture#382, allows
to create an infrastructure suitable for adoption testing.
@cjeanner cjeanner force-pushed the adoption/infra_playbook branch 2 times, most recently from 5a0705d to c34be3e Compare August 30, 2024 12:58
cjeanner added a commit to openstack-k8s-operators/ci-framework that referenced this pull request Aug 30, 2024
This playbook allows to create an infrastructure, mostly related to the
"adoption" use-case.

It allows to consume overrides provided by the automation file shipped
within the architecture repository.

This, alongside
openstack-k8s-operators/architecture#382, allows
to create an infrastructure suitable for adoption testing.
cjeanner added a commit to openstack-k8s-operators/ci-framework that referenced this pull request Sep 2, 2024
This playbook allows to create an infrastructure, mostly related to the
"adoption" use-case.

It allows to consume overrides provided by the automation file shipped
within the architecture repository.

This, alongside
openstack-k8s-operators/architecture#382, allows
to create an infrastructure suitable for adoption testing.
automation/vars/hci.yaml Outdated Show resolved Hide resolved
cjeanner added a commit to openstack-k8s-operators/ci-framework that referenced this pull request Sep 2, 2024
This playbook allows to create an infrastructure, mostly related to the
"adoption" use-case.

It allows to consume overrides provided by the automation file shipped
within the architecture repository.

This, alongside
openstack-k8s-operators/architecture#382, allows
to create an infrastructure suitable for adoption testing.
#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 cjeanner force-pushed the adoption/infra_playbook branch 2 times, most recently from 36860a8 to 02b880c Compare September 3, 2024 09:01
@cjeanner cjeanner changed the title Introduce "OSP" infra snippet in the automation file Introduce "adoption" snippet in automation file Sep 3, 2024
For adoption, there's a need to override the infra exposed by the
scenario.
While this new "adoption" entry creates a stronger link with the
ci-framework, it can also be consumed by any other tool, as long as it
knows how to consume the two "patches".

This is part of a PoC, and will probably lead to discussions regarding
the best way to manage all of the datasets.
amount: 0
osp-undercloud:
amount: 1
disk_file_name: rhel-8.10.qcow2
Copy link
Contributor

@fmount fmount Sep 3, 2024

Choose a reason for hiding this comment

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

I'm wondering if we want this level of detail in architecture, where we're defining a bunch of VMs settings.
I would have expected to have this kind of definition in ci-framework (like we do for greenfield, right?) and reference a "scenario" that can be loaded in terms of specific parameters.
However, I'm not saying this is a bad approach, as we have a clear view of the infra associated to VA1 in the adoption context in architectures.
I'll let @marios @jistr @cescgina comment further on this.

Copy link
Contributor Author

@cjeanner cjeanner Sep 3, 2024

Choose a reason for hiding this comment

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

we can. It could point to a thing to inherit from, but there are some caveats there, like the file location (remote vs ansible-controller). Indeed, this proposal here might link a bit too much to the framework, while having a file pointer could do.

And if we go down that path, we can push everything in that file - leading to this automation, in architecture, to have "just" adoption: relative/path/to/scenario.yml
But there would still be the "where to store t-h-t static/dynamic content that needs to be injected"... And there, I'm sure it doesn't belong to the ci-framework. Probably "bad" for architecture. But I'm not sure if there would be any other "good" place :/

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that a VA/DT already defines a set of services and it can be used as source of truth to define a static set of THT env files. That part is not "strictly" connected to the infra IMO, and it might live here in terms of "description" of the source cloud.

WRT the infra part, we could minimize the changes here by referencing a scenario that lives in the adoption ci-framework path, but I'd like to hear more opinions.
My comment started from the assumption that for greenfield VA/DTs we have scenario files that live in ci-framework, and we might want to be consistent with the already existing approach.

Copy link
Contributor

Choose a reason for hiding this comment

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

What if this infra style change to hci.yaml were in ci-framework/scenarios/adoption/hci.yaml?

Then in architecture/automation/vars/hci.yaml it could have a reference to ci-framework/scenarios/adoption/hci.yaml?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we could, indeed. We'd even NOT need any reference from any file: the scenario name, and the fact it's "adoption" should be sufficient to built the pointer to the file.

It won't solve the "where do we put OSP t-h-t content", but it reduces the amount of affected repositories in case of changes. I didn't think about it, and I'm ashamed of that ;_;.

Far, far cleaner to pass via scenario files - and that's the purpose if them anyway!

# Adoption related content for adoption
adoption:
hci:
layout_patch:
Copy link

Choose a reason for hiding this comment

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

Just diving a bit deeper into CI framework (this is my first real encounter with it).

If this is patching cifmw_libvirt_manager_layout then should we name the patch variables accordingly? Like cifmw_libvirt_manager_layout_patch? I wouldn't be afraid of long names for the sake of clarity.

Also where does the actual cifmw_libvirt_manager_layout value come from for the architecture jobs? I was wondering about the patching approach in general, vs. just redefining the variable for adoption, but i can't find the variable for greenfield here in this repo or in ci-framework. Is there another repo i should be looking at?

Copy link

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The patching is done here:
https://github.com/openstack-k8s-operators/ci-framework/blob/main/roles/libvirt_manager/tasks/generate_layout.yml

patches are nice, since we don't need to redefine the whole thing. There's one weakness though : we can't delete any entry.

The playbooks I proposed in openstack-k8s-operators/ci-framework#2285 are taking care of the dataset manipulation/patching

@cjeanner
Copy link
Contributor Author

cjeanner commented Sep 4, 2024

Closing this PR, since we can now rely on the ci-framework scenario nested in scenarios/adoption/ tree, at least for the adoption related overrides.

You can check openstack-k8s-operators/ci-framework#2285 to follow the evolution.

@cjeanner cjeanner closed this Sep 4, 2024
This pull request was closed.
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.

5 participants