CORS-3142: capi: write manifests to disk during create manifests #8166
Conversation
|
@r4f4: This pull request references CORS-3142 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.16.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci-robot
added
the
jira/valid-reference
|
Example run: |
|
/cc @patrickdillon |
|
Update: fixed unit tests. |
|
/assign @vincepri As @r4f4 mentioned this PR is placing machine manifests in a subdir of $ openshift-install create manifests --dir c
$ tree c -d
c
├── cluster-api
│ └── machines
├── manifests
└── openshift
$ tree c/cluster-api
c/cluster-api
├── 000_capi-namespace.yaml
├── 01_capi-cluster.yaml
├── 02_gcp-cluster.yaml
└── machines
├── 10_inframachine_padillon-gcp-capi-poc-gljxr-bootstrap.yaml
├── 10_inframachine_padillon-gcp-capi-poc-gljxr-master-0.yaml
├── 10_inframachine_padillon-gcp-capi-poc-gljxr-master-1.yaml
├── 10_inframachine_padillon-gcp-capi-poc-gljxr-master-2.yaml
├── 10_machine_padillon-gcp-capi-poc-gljxr-bootstrap.yaml
├── 10_machine_padillon-gcp-capi-poc-gljxr-master-0.yaml
├── 10_machine_padillon-gcp-capi-poc-gljxr-master-1.yaml
└── 10_machine_padillon-gcp-capi-poc-gljxr-master-2.yaml |
|
The commit message for 2cf920d says:
I think I may have confused this for you. I'm not aware of any issues that a |
|
/approve This LGTM. CI tests do run /test altinfra-e2e-aws-capi-ovn altinfra-e2e-openstack-capi-ovn altinfra-e2e-azure-capi-ovn altinfra-e2e-vsphere-capi-ovn |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: patrickdillon 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 |
openshift-ci
bot
added
the
approved
This will make it easier to load previously generated machine manifests later on. Right now, both the capi cluster manifest and the machine manifests `asset.Load` functions just read all yaml/json in cluster-api/. For example, trying to `create cluster` after `create manifests` as-is would result in the following error: ``` ERROR failed to fetch Cluster: failed to generate asset "Cluster": failed to create cluster: failed to create control-plane manifest: namespaces "openshift-cluster-api-guests" already exists ``` I have chosen to move the machine manifests to `cluster-api/machines` which shouldn't be technically problematic.
Without this meta type information, capi won't know hown the unmarshall
a serialized manifest, resulting in the following error:
```
FATAL failed to fetch Cluster: failed to load asset "Cluster API Manifests": failed to unmarshal file: error unmarshaling JSON: while decoding JSON: Object 'Kind' is missing in '{"metadata":{"creationTimestamp":null,"name":"openshift-cluster-api-guests"},"spec":{},"status":{}}'
```
Instead of hardcoding values which might easily be forgotten to be updated when the library version is bumped.
Write CAPI manifests to disk during create manifests so that they can be user edited and users can also provide their own set of manifests. In general, we think of manifests as an escape hatch that should be used when a feature is missing from the install config, and users accept the degraded user experience of editing manifests in order to achieve non-install-config-supported functionality.
|
Update: changed commit message as requested. |
|
/test altinfra-e2e-openstack-capi-ovn altinfra-e2e-azure-capi-ovn altinfra-e2e-vsphere-capi-ovn |
|
/test ? |
|
@r4f4: The following commands are available to trigger required jobs:
The following commands are available to trigger optional jobs:
Use
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Not sure why. I'll have to investigate. |
The `asset.PersistToFile` function already takes care of creating the necessary directories and files with appropriate permissions.
|
/test altinfra-e2e-openstack-capi-ovn altinfra-e2e-azure-capi-ovn altinfra-e2e-vsphere-capi-ovn |
|
Update: fixed a permission issue with creating the capi manifest directories From the sh-4.4$ ls -l /tmp/installer/cluster-api/*
-rw-r-----. 1 1005000000 root 124 Mar 15 19:41 /tmp/installer/cluster-api/000_capi-namespace.yaml
-rw-r-----. 1 1005000000 root 107 Mar 15 19:41 /tmp/installer/cluster-api/00_azure-namespace.yaml
-rw-r-----. 1 1005000000 root 510 Mar 15 19:41 /tmp/installer/cluster-api/01_aws-cluster-controller-identity-default.yaml
-rw-r-----. 1 1005000000 root 230 Mar 15 19:41 /tmp/installer/cluster-api/01_azure-client-secret.yaml
-rw-r-----. 1 1005000000 root 464 Mar 15 19:41 /tmp/installer/cluster-api/01_capi-cluster.yaml
-rw-r-----. 1 1005000000 root 1168 Mar 15 19:41 /tmp/installer/cluster-api/02_azure-cluster.yaml
/tmp/installer/cluster-api/machines:
total 32
-rw-r-----. 1 1005000000 root 840 Mar 15 19:41 10_inframachine_ci-op-qtzzzlk5-bdefc-8tq79-bootstrap.yaml
-rw-r-----. 1 1005000000 root 775 Mar 15 19:41 10_inframachine_ci-op-qtzzzlk5-bdefc-8tq79-master-0.yaml
-rw-r-----. 1 1005000000 root 775 Mar 15 19:41 10_inframachine_ci-op-qtzzzlk5-bdefc-8tq79-master-1.yaml
-rw-r-----. 1 1005000000 root 775 Mar 15 19:41 10_inframachine_ci-op-qtzzzlk5-bdefc-8tq79-master-2.yaml
-rw-r-----. 1 1005000000 root 507 Mar 15 19:41 10_machine_ci-op-qtzzzlk5-bdefc-8tq79-bootstrap.yaml
-rw-r-----. 1 1005000000 root 502 Mar 15 19:41 10_machine_ci-op-qtzzzlk5-bdefc-8tq79-master-0.yaml
-rw-r-----. 1 1005000000 root 502 Mar 15 19:41 10_machine_ci-op-qtzzzlk5-bdefc-8tq79-master-1.yaml
-rw-r-----. 1 1005000000 root 502 Mar 15 19:41 10_machine_ci-op-qtzzzlk5-bdefc-8tq79-master-2.yaml |
|
/lgtm |
|
/label acknowledge-critical-fixes-only |
openshift-ci
bot
added
the
acknowledge-critical-fixes-only
|
/test e2e-openstack-ovn |
|
@r4f4: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
/override ci/prow/e2e-openstack-ovn |
|
@r4f4: Overrode contexts on behalf of r4f4: ci/prow/e2e-openstack-ovn In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
openshift-merge-bot
bot
merged commit fb92a96
into
openshift:master
|
[ART PR BUILD NOTIFIER] This PR has been included in build ose-installer-altinfra-container-v4.16.0-202403201847.p0.gfb92a96.assembly.stream.el8 for distgit ose-installer-altinfra. |
This PR:
.GroupVersion.String()to retrieveAPIVersionfrom the respective libs.create manifests