Skip to content

Commit

Permalink
Merge branch 'main' into add_worker_unidelta_ipv6
Browse files Browse the repository at this point in the history
  • Loading branch information
gmarcian authored Nov 19, 2024
2 parents 52f9c6c + d60373f commit 88e97a0
Show file tree
Hide file tree
Showing 10 changed files with 129 additions and 51 deletions.
105 changes: 62 additions & 43 deletions docs/contributing/common-pitfalls.md
Original file line number Diff line number Diff line change
@@ -1,62 +1,81 @@
# Common Design Pitfalls

This document lists common pitfalls that have been observed in the process of creating
and modifying VAs and DTs.
This document lists common pitfalls that have been observed in the process of
creating and modifying VAs and DTs.

## Accidental OpenStackControlPlane Overwrites

In general, it is a best practice to keep all kustomizations (`patches`, `replacements`, etc)
for a particular resource in one `kustomization.yaml` file. Sometimes, however, it is
necessary to only perform a subset of `OpenStackControlPlane` kustomizations at a certain stage
of the deployment process. For instance, you might not want to kustomize an `OpenStackControlPlane`
CR with certain data during its initial creation stage because that data is not yet available
for use. Thus it would make sense to have a later stage and `kustomization.yaml` file to
add those kustomzations once the requisite data is available (perhaps during the data plane
deployment stage).

**What is crucial to keep in mind is that any kustomizations to a resource in an earlier stage
will be lost/overwritten in later stages where that same resource is modified _if_ those stages
do not reference the same `kustomization.yaml` that the earlier stage utilized.** Thus it is
best to have a base `kustomization.yaml` for a given resource for all kustomizations common to
all stages -- and all those stages should thus reference that `kustomization.yaml`. Then, if
later stages need specific changes for that resource, a separate `kustomization.yaml` can be also
used to apply those additional kustomizations beyond the base ones. This approach is also
preferred to creating two somewhat-or-mostly duplicate `kustomization.yaml`s, one for the earlier
stage and one for a later stage. Keeping things DRY by using a common base will make future
potential changes to the `kustomization.yaml`s less prone to error, as changes to the common file
will automatically be picked up by all deployment stages.

As an illustrative example of the best practice mentioned above, consider the following directory
structure:
In general, it is a best practice to keep all kustomizations (`patches`,
`replacements`, etc) for a particular resource in one `kustomization.yaml`
file.

In some cases it is necessary to only perform a subset of
`OpenStackControlPlane` kustomizations at a certain stage of the deployment
process. For instance, you might not want to kustomize an
`OpenStackControlPlane` CR with certain data during its initial creation stage
because that data is not yet available for use. In the case of a multi-stage
deployment, it would make sense to have a separate `kustomization.yaml` file to
add those kustomizations once the requisite data is available (perhaps during
the data plane deployment stage).

**What is crucial to keep in mind is that any kustomizations to a resource in
an earlier stage will be lost/overwritten in later stages where that same
resource is modified _if_ those stages do not reference the same
`kustomization.yaml` that the earlier stage utilized.**

It is best to have a base `kustomization.yaml` for a given resource for all
kustomizations common to all stages -- and all those stages should reference
that `kustomization.yaml`. If later stages need specific changes for that
resource, a separate `kustomization.yaml` can used to apply those additional
kustomizations beyond the base ones.

The use of common base files is preferred to creating two nearly-identical
`kustomization.yaml` files; one for the earlier stage and one for a later
stage. Keeping things DRY by using a common base will make future potential
changes to the `kustomization.yaml` files less prone to error, as changes to
the common file will automatically be picked up by all deployment stages.

As an illustrative example of the best practice mentioned above, consider the
following directory structure:

```
some_dt_or_va/control_plane/kustomization.yaml
some_dt_or_va/data_plane/kustomization.yaml
```

If the `data_plane/kustomization.yaml` needs to modify the `OpenStackControlPlane`, then it should
reference `../control_plane/kustomization.yaml` as a `Component` and then add additional `replacements`
and/or `patches` as needed. If it were to instead reference this repo's [lib/control-plane](../../lib/control-plane)
directory as its base `OpenStackControlPlane` `Component`, then the `../control_plane/kustomization.yaml`
kustomizations would be lost, since the `OpenStackControlPlane` CR would be generated and applied without
them.
If the `data_plane/kustomization.yaml` needs to modify the
`OpenStackControlPlane`, then it should reference
`../control_plane/kustomization.yaml` as a `Component` and then add additional
`replacements` and/or `patches` as needed.

If it were to instead reference this repositories
[lib/control-plane](../../lib/control-plane) directory as its base
`OpenStackControlPlane` `Component`, then the
`../control_plane/kustomization.yaml` kustomizations would be lost, since the
`OpenStackControlPlane` CR would be generated and applied without them.

It also follows in this scenario that, as mentioned above, the `OpenStackControlPlane` kustomizations for
its initial creation stage should be located in one and only one `kustomization.yaml`. Thus you would
want to avoid something like this...
The kustomizations for an `OpenStackControlPlane` resource should be within a
single `kustomization.yaml` that contains the kustomizations for the initial
creation stage. You want to avoid the use of multiple files, such as creating
an additional sub-directory within the same base directory containing the
configuration. The following would be an example to avoid:

```
some_dt_or_va/control_plane/kustomization.yaml
some_dt_or_va/control_plane/some_subdir/kustomization.yaml
some_dt_or_va/data_plane/kustomization.yaml
```

..._if_ `some_dt_or_va/control_plane/some_subdir/kustomization.yaml` has further kustomizations to the
`OpenStackControlPlane` beyond `some_dt_or_va/control_plane/kustomization.yaml`. (It would be fine, for
instance, if that subdirectory was modifying some other resource like `NodeNetworkConfigurationPolicy`).
The reason for this is again that, if later stages do not want to accidentally overwrite earlier
`OpenStackControlPlane` kustomizations, those later stages will need to reference both
`../control_plane/kustomization.yaml` and `../control_plane/some_subdir/kustomization.yaml` in the case
that those stages are modifying the `OpenStackControlPlane`. It would be better for the two directories
to be collapsed into one, such that a single `kustomization.yaml` can be referenced as a `Component` to
include all the previous stage's kustomizations and not inadvertently overwrite them.
In some cases having an additional nested directory may be valid, in the case a
subdirectory was modifying some other resource like
`NodeNetworkConfigurationPolicy`.

If later stages do not want to accidentally overwrite earlier
`OpenStackControlPlane` kustomizations, those later stages need to reference
both `../control_plane/kustomization.yaml` and
`../control_plane/some_subdir/kustomization.yaml` so that those stages are
modifying the `OpenStackControlPlane`.

It would be better for the two directories to be collapsed into one, such that
a single `kustomization.yaml` can be referenced as a `Component` to include all
the previous stage's kustomizations and not inadvertently overwrite them.
11 changes: 11 additions & 0 deletions dt/dcn/edpm-post-ceph/nodeset/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,17 @@ replacements:
- spec.neutron.template.customServiceConfig
options:
create: true
- source:
kind: ConfigMap
name: service-values
fieldPath: data.ovn.template.ovnController.external-ids
targets:
- select:
kind: OpenStackControlPlane
fieldPaths:
- spec.ovn.template.ovnController.external-ids
options:
create: true
- source:
kind: ConfigMap
name: service-values
Expand Down
36 changes: 36 additions & 0 deletions dt/osasinfra/edpm-post-ceph/nodeset/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -310,3 +310,39 @@ replacements:
- spec.ovn.template.ovnController.nicMappings
options:
create: true

- source:
kind: ConfigMap
name: service-values
fieldPath: data.telemetry.enabled
targets:
- select:
kind: OpenStackControlPlane
fieldPaths:
- spec.telemetry.enabled
options:
create: true

- source:
kind: ConfigMap
name: service-values
fieldPath: data.telemetry.metricStorage.enabled
targets:
- select:
kind: OpenStackControlPlane
fieldPaths:
- spec.telemetry.template.metricStorage.enabled
options:
create: true

- source:
kind: ConfigMap
name: service-values
fieldPath: data.telemetry.ceilometer.enabled
targets:
- select:
kind: OpenStackControlPlane
fieldPaths:
- spec.telemetry.template.ceilometer.enabled
options:
create: true
4 changes: 0 additions & 4 deletions examples/dt/bmo01/control-plane/nncp/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@ metadata:
data:
openstack-operator-image: "quay.io/openstack-k8s-operators/openstack-operator-index:latest"

ocp:
cluster_network_cidr: 192.168.16.0/20
service_network_cidr: 172.30.0.0/16

node_0:
name: master-0
internalapi_ip: 172.17.0.10
Expand Down
2 changes: 2 additions & 0 deletions examples/dt/dcn/edpm-pre-ceph/nodeset/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ data:
gather_facts: false
neutron_physical_bridge_name: br-ctl
neutron_public_interface_name: eth0
edpm_enable_chassis_gw: false
edpm_ovn_availability_zones: []
edpm_ceph_hci_pre_enabled_services:
- ceph_mon
- ceph_mgr
Expand Down
9 changes: 9 additions & 0 deletions examples/dt/dcn/service-values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,15 @@ data:
network_vlan_ranges = datacentre:1:1000,leaf1:1:1000,leaf2:1:1000
[neutron]
physnets = datacentre,leaf1,leaf2
ovn:
template:
ovnController:
external-ids:
availability-zones: []
enable-chassis-as-gateway: true
ovn-bridge: br-int
ovn-encap-type: geneve
system-id: random
nova:
customServiceConfig: |
[DEFAULT]
Expand Down
1 change: 1 addition & 0 deletions examples/dt/osasinfra/control-plane.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## Assumptions

- A storage class called `local-storage` should already exist.
- Cluster observability operator is already deployed.

## Initialize

Expand Down
7 changes: 7 additions & 0 deletions examples/dt/osasinfra/service-values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -113,3 +113,10 @@ data:
nicMappings:
datacentre: ocpbr
octavia: octbr

telemetry:
enabled: true
metricStorage:
enabled: true
ceilometer:
enabled: true
1 change: 1 addition & 0 deletions examples/dt/osasinfra/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ data:
- neutron-metadata
- libvirt
- nova
- telemetry
ceph:
conf: CHANGEME_CEPH_CONF
keyring: CHANGEME_CEPH_KEYRING
4 changes: 0 additions & 4 deletions examples/dt/uni01alpha/control-plane/nncp/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@ metadata:
data:
openstack-operator-image: "quay.io/openstack-k8s-operators/openstack-operator-index:latest"

ocp:
cluster_network_cidr: 192.168.16.0/20
service_network_cidr: 172.30.0.0/16

node_0:
name: master-0
internalapi_ip: 172.17.0.10
Expand Down

0 comments on commit 88e97a0

Please sign in to comment.