Add support for new platform installation method of AAP 2.5 on OpenShift #261
Conversation
|
I'm planning on reviewing and testing, I'll should be able to get to it in the next day or two. |
There was a problem hiding this comment.
@derekwaters Thank you for your patience with me reviewing this PR.
I'd liek to request a couple of changes. I've been to get a working installation, but so far have not been successful. The Operator is installed and working and I've been able to get a platform manifest created and it started with a controller defined, but for several minutes after the playbook completes the web ui fails during login (I think because the controller instance is not ready yet). So I think there needs to be some other checks to ensure that the system is ready to be used before the playbook ends.
Also, it would be good to support installing into a different namespace than the operator is installed in.
I'm still getting my head around how the 2.5 install works and will provide more feedback.
One last request is to add an example playbook for installing the 2.5 version. Maybe relabel the existing example to be 2.4 and below and the new example as 2.5 and above.
- Wait until controller/eda/hub APIs are available before marking installation complete
|
This PR has been updated to include:
|
|
@branic have you had time to test it? |
There was a problem hiding this comment.
@derekwaters This PR is looking good. I like the changes you've made after the last review. I've got a few more comments and requests and I think this PR will be good to merge.
P.S. Sorry for not seeing this sooner and getting the review done.
P.P.S @djdanielsson thanks for the ping.
| @@ -78,6 +83,13 @@ If the variable is omitted the corresponding component will not be installed (e. | |||
| | link_text | | Automation Hub (<INSTANCE_NAME>) | Text used for creating the OCP application link | | |||
| | hub_manifest_overrides | | None | YAML Manifest to override the generated `AutomationHub` resource | | |||
| | consolelink_manifest_overrides | | None | YAML Manifest to override the generated `ConsoleLink` resource | | |||
| | storage_type | | file | Hub storage type (file, S3 or azure)* | | |||
There was a problem hiding this comment.
The default value isn't being taken into account. For example if specifying an instance name like:
aap_ocp_install_hub:
instance_name: tst-hub
The an error is encountered that the storage_type must be set:
TASK [infra.aap_utilities.aap_ocp_install : Ensure no validation errors found] ****************************************************************************
fatal: [localhost]: FAILED! => {
"msg": [
"The following errors must be fixed to continue:",
[
"aap_ocp_install_hub['storage_type'] must be 'file', 'S3' or 'azure'"
]
]
}
| | storage_type | | file | Hub storage type (file, S3 or azure)* | | ||
| | file_storage_storage_class | | None | OpenShift StorageClass to use for file storage type for hub* | | ||
| | file_storage_size | | 10Gi | Storage size for file storage type for hub* | | ||
| | object_storage_s3_secret | | None | Name of an OpenShift Secret used to access S3 storage for hub* | | ||
| | object_storage_azure_secret | | None | Name of an OpenShift Secret used to access Azure storage for hub* | |
There was a problem hiding this comment.
I think it would be clearer (and match other parts of the readme) if the * was moved to the Required column instead of being at the end of the description.
| @@ -40,12 +43,14 @@ If the variable is omitted the corresponding component will not be installed (e. | |||
|
|
|||
| | Key Name | Required | Default Value | Description | | |||
| |----------------------------------|:---------:|---------------|---------------------------------------------------------------------| | |||
| | channel | Yes | None | Channel to subscribe (e.g. stable-2.2 or stable-2.2-cluster-scoped) | | |||
| | channel | Yes | None | Channel to subscribe (e.g. stable-2.2 or stable-2.2-cluster-scoped)*| | |||
There was a problem hiding this comment.
I think it would be clearer (and match other parts of the readme) if the * was moved to the Required column instead of being at the end of the description.
NOTE: This applies to other sections where the * is in the description column.
| > ℹ️ **NOTE** | ||
| > | ||
| > These settings are only used when installing AAP 2.5 or later. instance_name and link_text values for individual components (hub, controller, eda) will be ignored when using the platform installation method. | ||
|
|
There was a problem hiding this comment.
A similar * note should also be made for the instance_name and link_text name in the tables for each component key.
Also, this note and notes in other sections needs to include namespace as well as that is ignored in a platform install and all installed components are installed in the platform namespace.
| Including an example of how to use your role (for instance, with variables passed in as parameters) is always nice for users too. | ||
|
|
There was a problem hiding this comment.
Let's just get rid of these two lines. I don't think they are necessary and are just from the template.
| - name: Ensure automation hub API is available | ||
| ansible.builtin.uri: | ||
| url: "https://{{ __aap_ocp_install_hub_route }}/api" | ||
| validate_certs: "{{ aap_ocp_install_connection['validate_certs'] | default(omit) }}" | ||
| method: GET | ||
| status_code: | ||
| - 200 | ||
| when: | ||
| - aap_ocp_install_hub is defined | ||
| register: _aap_ocp_install_hub_available | ||
| until: (_aap_ocp_install_hub_available['status'] == 200) and ('migrations_notran' not in _aap_ocp_install_hub_available['url']) | ||
| retries: 120 # Wait for 30 minutes (120*15/60) | ||
| delay: 15 |
There was a problem hiding this comment.
This check is failing for me. Doing a get of the Hub route with /api returns a status_code of 404. However, using a path of /api/galaxy/pulp/api/v3/ returns a 200 status_code and allows the task to complete.
| - name: Set a flag indicating whether to use the new (AAP 2.5+) operator installation method | ||
| ansible.builtin.set_fact: | ||
| __aap_ocp_install_25_install: "{{ (__aap_ocp_install_major_version | int) > 2 \ | ||
| or (( __aap_ocp_install_major_version | int) == 2 \ |
There was a problem hiding this comment.
To fix an ansible-lint warning:
| or (( __aap_ocp_install_major_version | int) == 2 \ | |
| or ((__aap_ocp_install_major_version | int) == 2 \ |
| ansible.builtin.set_fact: | ||
| __aap_ocp_install_25_install: "{{ (__aap_ocp_install_major_version | int) > 2 \ | ||
| or (( __aap_ocp_install_major_version | int) == 2 \ | ||
| and ( __aap_ocp_install_minor_version | int) >= 5) }}" |
There was a problem hiding this comment.
To fix an ansible-lint warning:
| and ( __aap_ocp_install_minor_version | int) >= 5) }}" | |
| and (__aap_ocp_install_minor_version | int) >= 5) }}" |
| @@ -0,0 +1,2 @@ | |||
| minor_changes: | |||
| - Added ability to install EDA Controller on OCP | |||
There was a problem hiding this comment.
This changelog fragment and filename don't describe what change this PR is actually making.
| {{ __aap_ocp_install_prevalidate_errors + ["aap_ocp_install_controller['projects_storage_size'] must be a number greater than 0 with a size (e.g. 12Gi or 10000M)"] }} | ||
|
|
||
| - name: Ensure hub storage type is valid (block) | ||
| when: aap_ocp_install_hub is defined |
There was a problem hiding this comment.
If the intention is to default the storage_type to file (which I think is a good default). Then this when clause should only be checked when a storage_type is defined.
| when: aap_ocp_install_hub is defined | |
| when: aap_ocp_install_hub['storage_type'] is defined |
What does this PR do?
This PR modifies the aap_ocp_install role to support the new platform installation method introduced to the AAP operator in AAP 2.5. It is backward
How should this be tested?
Requires an OpenShift cluster with appropriate credentials, then run:
with configuration as specified in the README and vars files defined in the role.
Once complete, installation can be validated by accessing AAP via the quick links toolbar menu option.
Is there a relevant Issue open for this?
#260
resolves #260
Other Relevant info, PRs, etc
None