Add create and teardown cluster playbooks - #2
adriengentil merged 11 commits into
Conversation
91cea3d to
0d31709
Compare
adriengentil
changed the title
0d31709 to
ce49052
Compare
| - name: Create ManagedCluster resource | ||
| kubernetes.core.k8s: | ||
| state: "{{ hosted_cluster_state }}" | ||
| definition: | ||
| apiVersion: cluster.open-cluster-management.io/v1 | ||
| kind: ManagedCluster | ||
| metadata: | ||
| name: "{{ hosted_cluster_name }}" | ||
| namespace: "{{ hosted_cluster_namespace }}" | ||
| annotations: | ||
| import.open-cluster-management.io/hosting-cluster-name: "{{ hosted_cluster_name }}" | ||
| import.open-cluster-management.io/klusterlet-deploy-mode: Hosted | ||
| open-cluster-management/created-via: hypershift | ||
| labels: | ||
| name: "{{ hosted_cluster_name }}" | ||
| cloud: BareMetal | ||
| vendor: OpenShift | ||
| cluster.open-cluster-management.io/clusterset: default | ||
| spec: | ||
| hubAcceptsClient: true |
There was a problem hiding this comment.
Do we to create this in the playbook? In the past when I've deployed a hostedcluster, I haven't needed to create the ManagedCluster resource explicitly.
There was a problem hiding this comment.
Not sure, I basically copy-pasted what the OCM UI does when creating a hosted cluster. I'm happy to remove it if it's not needed.
There was a problem hiding this comment.
I see this manifest mentioned here in this blog post: https://www.redhat.com/en/blog/how-to-build-bare-metal-hosted-clusters-on-red-hat-advanced-cluster-management-for-kubernetes
| platform: | ||
| type: Agent | ||
| agent: | ||
| agentNamespace: '' |
There was a problem hiding this comment.
Doesn't this need to be set to the namespace that contains the target infraenv?
| agent: | ||
| agentLabelSelector: | ||
| matchLabels: | ||
| infraenvs.agent-install.openshift.io: "{{ hosted_cluster_template.infraenv }}" |
There was a problem hiding this comment.
Flagging this for discussion, because I think it's odd that we have ended up with infraenv as a template parameter. I think -- at least for POC1, and possibly going forward -- that we will also want to match agents on cluster name, and some other component will be responsible for assigning agents to a cluster (e.g., we would do that by hand for POC1).
There was a problem hiding this comment.
I agree that something more functional would be great.
About the var file used as a template, I was thinking about the parameters section to be merged with the parameters provided by the user in the cluster order, so everything outside this section would be hard-coded by the template.
Hard-code the agent namespace to "hardware-inventory" until we find an aggreement on how to get them properly.
45e3d96 to
655f265
Compare
fe10e1b to
439eb76
Compare
439eb76 to
5cf5aea
Compare
| rules: | ||
| - name: Create hosted cluster | ||
| condition: event.payload.message == "create_hosted_cluster" | ||
| condition: event.meta.endpoint == "create-hosted-cluster" |
There was a problem hiding this comment.
@larsks I updated the rulebook to route the event based on the URL path:
<eda url>/create-hosted-clusterwill trigger the cluster creation<eda url>/delete-hosted-clusterwill trigger the cluster deletion
split the hosted_cluster roles in order to insolate the logic that determines in which namespace we should create the cluster object should be created, and the logic that computes the cluster settings based on a default template and user-provided parameters.
482ea21 to
b70f54a
Compare
WalkthroughThe pull request introduces several updates across configuration files, playbooks, and role definitions. Changes include additions to the 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (5)
🔇 Additional comments (5)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (9)
README.md (2)
10-13: Specify Language for Fenced Code BlockThe fenced code block that shows the commands (
$ uv syncand$ source .venv/bin/activate) does not specify a language. Adding a language identifier (e.g.,bash) would help satisfy markdownlint MD040 and improve readability.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
10-10: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
11-11: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
12-12: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
17-19: Include Language Identifier for Command BlockSimilarly, the fenced code block starting at line 17 (with the ansible-galaxy command) would benefit from a language specifier (e.g.,
bash) to comply with markdownlint guidelines (MD040, MD014) and to better indicate that these are shell commands rather than plain text.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
17-17: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
18-18: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
roles/cluster_settings/defaults/main.yml (1)
1-4: Define Cluster Settings DefaultsThe YAML file clearly defines the default credentials secret name and namespace. For enhanced maintainability, consider adding brief inline comments to explain the purpose of each variable.
roles/hosted_cluster/defaults/main.yml (1)
1-6: Configure Hosted Cluster DefaultsThe default settings for the hosted cluster role are defined effectively. The use of a Jinja template variable (
{{ hosted_cluster_name }}) in the label selector is appropriate. Optionally, inline comments describing what these defaults represent could further improve clarity.roles/cluster_settings/vars/ocp.4-17.small.yml (1)
1-8: Set Up OCP 4.17 Small Cluster SettingsThis configuration file sets the cluster settings for an OpenShift Container Platform (OCP) 4.17 small cluster variant clearly. Adding inline comments to briefly describe the purpose of sections like
cluster_settings_resultandcluster_settings_template_defaultsmay be beneficial for future maintainers.pyproject.toml (1)
1-11: Project Configuration and MetadataThe
pyproject.tomlfile correctly sets the project metadata, including dependencies and required Python version. The configuration is clear; however, consider expanding the description field to provide a more detailed summary of the project’s purpose, which would help new contributors.roles/cluster_settings/tasks/main.yml (1)
16-25: Nitpick: Simplify Jinja expressions in combine filter.
Instead of nesting Jinja templating within the combine filter, consider directly referencing the variables. This improves readability.Example diff:
- {{ cluster_settings_template_defaults | combine({ - 'pull_secret': '{{ default_cluster_credentials.resources[0].data.pull_secret }}', - 'ssh_key': '{{ default_cluster_credentials.resources[0].data.pull_secret_pub }}' - }) - }} + {{ cluster_settings_template_defaults | combine({ + 'pull_secret': default_cluster_credentials.resources[0].data.pull_secret, + 'ssh_key': default_cluster_credentials.resources[0].data.ssh_key_pub + }) + }}roles/cluster_working_namespace/tasks/main.yml (1)
2-8: Nitpick: Correct typo in task name.
Consider changing “CusterOrder” to “ClusterOrder” in the task title for clarity.Example diff:
- - name: Retrieve working cluster namespace from CusterOrder name and namespace + - name: Retrieve working cluster namespace from ClusterOrder name and namespaceroles/hosted_cluster/tasks/main.yml (1)
2-8: Typo in Task Name
There is a minor typo in the task title at line 2: “Manage Secret resource containning pull-secret” should be corrected to “Manage Secret resource containing pull-secret” for clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
.gitignore(1 hunks).python-version(1 hunks)README.md(1 hunks)ansible.cfg(1 hunks)collections/requirements.yml(1 hunks)playbook_cloudkit_create_hosted_cluster.yml(1 hunks)playbook_cloudkit_delete_hosted_cluster.yml(1 hunks)pyproject.toml(1 hunks)roles/cluster_settings/defaults/main.yml(1 hunks)roles/cluster_settings/tasks/main.yml(1 hunks)roles/cluster_settings/vars/ocp.4-17.small.yml(1 hunks)roles/cluster_working_namespace/defaults/main.yml(1 hunks)roles/cluster_working_namespace/tasks/main.yml(1 hunks)roles/hosted_cluster/defaults/main.yml(1 hunks)roles/hosted_cluster/tasks/main.yml(1 hunks)rulebooks/cluster_fulfillment.yml(1 hunks)samples/hosted_cluster_extra_vars.yml(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
10-10: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
11-11: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
12-12: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
17-17: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
18-18: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
🔇 Additional comments (27)
.gitignore (1)
173-174: Good additions to .gitignoreAdding
.ansibleand.vscodeto the gitignore is appropriate as these directories contain local environment configurations that shouldn't be committed to the repository. This helps keep the repository clean of personal development environment files.ansible.cfg (1)
5-7: Appropriate Jinja2 configuration for working with Kubernetes manifestsThe addition of
jinja2_native=Trueis a good practice when generating Kubernetes manifests, as it ensures numeric values are rendered without quotes. This is particularly useful when working with the newly addedkubernetes.corecollection which may require specific data types in manifests.collections/requirements.yml (1)
2-4: Good addition of kubernetes.core collectionAdding the
kubernetes.corecollection as a dependency aligns well with the PR objectives of creating playbooks for hosted cluster management. Version 5.2.0 is a specific version which helps ensure consistency across environments.README.md (1)
6-8: Introduce "Pre-requisites" Section ClearlyThe new "Pre-requisites" section is well organized and clearly outlines the steps to set up the project environment using the
uvtool. This addition helps users understand the installation process.roles/cluster_working_namespace/defaults/main.yml (1)
1-3: LGTM: YAML defaults are correctly defined.
The variables for the cluster order labels are clear and align with the intended use in the playbooks and roles.samples/hosted_cluster_extra_vars.yml (2)
1-10: LGTM: Cluster order configuration is well structured.
Thecluster_orderblock properly defines metadata and specifications, making it clear how the payload should be structured.
11-13: LGTM: Default settings for credentials and environments.
The additional default variables for secret namespaces and infrastructure settings are defined properly and should integrate well with dependent roles.rulebooks/cluster_fulfillment.yml (2)
10-10: LGTM: Condition for creating hosted cluster is correct.
Usingevent.meta.endpoint == "create-hosted-cluster"complies with the new event structure and improves clarity.
17-17: LGTM: Condition for deleting hosted cluster is correct.
The update to checkevent.meta.endpoint == "delete-hosted-cluster"ensures consistency in event handling across rules.roles/cluster_settings/tasks/main.yml (4)
2-5: LGTM: Retrieval of default parameters is implemented correctly.
The task usesansible.builtin.include_varsto load the configuration based on the dynamic template ID. Ensure that the referenced file exists during runtime.
6-15: LGTM: Fetching default credentials is handled securely.
Usingkubernetes.core.k8s_infowith proper conditions andno_log: trueensures secure retrieval of sensitive data.
27-31: LGTM: Merging default and user-provided parameters.
The use of thecombinefilter to merge template parameters is clear and correctly implemented.
33-37: LGTM: Merging with read-only settings is implemented well.
This final merge ensures that the computed settings appropriately integrate both user inputs and system defaults.roles/cluster_working_namespace/tasks/main.yml (2)
10-14: LGTM: Proper failure handling when namespace is not uniquely found.
The conditional check using| length != 1is effective in ensuring a unique match, triggering an appropriate error message if not.
15-18: LGTM: Setting the working namespace result is clear and correct.
The fact is set from the first resource returned, correctly capturing the namespace name for subsequent tasks.playbook_cloudkit_create_hosted_cluster.yml (4)
2-3: Clarify Playbook Naming and Purpose
The updated name “Create a hosted control plane cluster from a ClusterOrder” clearly indicates the playbook’s intent.
6-7: Good Initialization ofcluster_orderVariable
Initializingcluster_orderwith the payload fromansible_eda.eventis clear and follows the intended design.
15-19: Configuration of Rolecluster_settingsis Clear
The template ID and template parameters are sourced directly from thecluster_order, which is consistent with the design objectives.
20-25: Good Usage of Dynamic Variables in thehosted_clusterRole
The hosted cluster role correctly receives its name fromcluster_order.metadata.nameand dynamically sets the namespace and settings viacluster_working_namespace_resultandcluster_settings_result. Assigninghosted_cluster_stateaspresentclearly indicates this is the create playbook.roles/hosted_cluster/tasks/main.yml (4)
2-15: Pull Secret Definition is Well-Configured
The secret resource for the pull secret is properly defined with the appropriate API version, kind, metadata, and data format. The use ofno_log: truehelps to prevent sensitive data exposure.
17-28: SSH Key Secret Task Looks Good
This task correctly defines the SSH key Secret with dynamic metadata and data fromhosted_cluster_settings. Since this secret is for an SSH public key, omittingno_logis acceptable.
29-78: HostedCluster Resource Task is Comprehensive
The manifest for the HostedCluster resource is detailed and leverages dynamic variables effectively for image, networking, platform, and service configurations. Ensure that the hard-coded network CIDRs and availability policies align with the operational requirements.
79-100: NodePool Resource Definition is Clear
The NodePool resource configuration correctly ties in the hosted cluster name, replica count (with an explicit conversion to int), and upgrade management. The use of dynamic variables provides flexibility in deployment.playbook_cloudkit_delete_hosted_cluster.yml (4)
2-3: Clear Playbook Renaming for Teardown
Changing the playbook name to “Teardown a hosted control plane cluster from a ClusterOrder” accurately reflects its purpose.
6-8: Consistent Variable Initialization
Initializingcluster_orderhere mirrors the create playbook and ensures consistency across the playbook designs.
15-19:cluster_settingsRole Configuration is Consistent
The task passes the template ID and parameters as expected from the cluster order, maintaining consistency with the create playbook.
20-25: Proper Configuration for Cluster Teardown
Within thehosted_clusterrole, the resources are set for deletion by configuringhosted_cluster_stateasabsent. Dynamic variables for name, namespace (fromcluster_working_namespace_result), and settings (fromcluster_settings_result) ensure that the proper resources are targeted during teardown.
b70f54a to
56ca53f
Compare
…rd compat, and storage provision tests - Replaces VAST_STORAGE_TIERS single env var approach with safe raw-string parsing: _vast_storage_tiers_raw in defaults, from_json in configure_backend with explicit empty-check assert for actionable error messages (finding osac-project#2) - Adds 5 tier validation tasks: non-empty list, SDR-016 type/mapping check, SDR-009 protocol enum, DNS label names, uniqueness, SDR-010 max_tiers guard - Moves credential resolution before CSI install (fail-fast, SDR-001 outer block/always clears 9 credential facts in all exit paths, SDR-017) - Rewrites ensure_csi_operator.yaml for multi-protocol: per-protocol CSIDriver check, _vast_protocols_needing_install derivation, single OLM Subscription (SDR-002 preserved), per-protocol VastCSIDriver CRs, per-protocol wait - Replaces single View/Quota tasks with per-tier loops (no_log, SDR-008); removes non-functional QoS view policy tasks - Adds Jinja2 comprehension _vast_sc_tiers, replace_default additive override, per-tier VolumeSnapshotClass (vast-snapshot-{tenant}-{tier} naming) - Updates tenant config Secret to store storage_tiers JSON (replaces storage_tier, storage_protocol, storage_path, view_policy_name fields) - Adds two-format reader to cleanup_backend: reads storage_tiers JSON if present, falls back to storage_tier/storage_protocol for pre-hardening Secrets; loops View and VSC deletions over tiers (finding osac-project#3) - Adds storage_provision integration test target: single-tier NFS, dual-tier NFS+block with quota, validation failures, replace_default (finding osac-project#1) - Updates configmap sample to VAST_STORAGE_TIERS JSON format - Updates secret-vast-tenant-config-test fixture to new Secret format Generated-By: Claude Code (Anthropic) Signed-off-by: Will Gordon <wgordon@redhat.com>
3 participants
This PR creates 2 playbooks to create and delete hosted cluster. To perform this action,I created 3 roles:
cluster_working_namespace: find and return the namespace name in which all the resources will be createdcluster_settings: contains the logic to compute the settings which will be used to create the hosted cluster, it basically merges settings set at template level the ones provided by the userhosted_cluster: creates the resources in the namespace returned bycluster_working_namespaceand use the settings returnedcluster_settingsto customize the deploymentSummary by CodeRabbit
Documentation
New Features
Chores