Skip to content

Conversation

bfournie
Copy link

@bfournie bfournie commented Jul 9, 2025

In the agent-based installer, when cluster installation is complete the assisted-installer cannot access assisted-service since it has terminated. Therefore any operator custom-manifests are not applied. Instead the assisted-service will create a ConfigMap with the manifests and operator info (openshift/assisted-service#7818) and in this PR the assisted-installer will retrieve the ConfigMap and apply the manifests.

The intent is that this will only be done for the agent-based installer and not affect the normal path of applying manifests using the assisted-service API.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 9, 2025
@openshift-ci-robot
Copy link

openshift-ci-robot commented Jul 9, 2025

@bfournie: This pull request references AGENT-1072 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.20.0" version, but no target version was set.

In response to this:

In the agent-based installer, when cluster installation is complete the assisted-installer cannot access assisted-service since it has terminated. Therefore any operator custom-manifests are not applied. Instead the assisted-service will create a ConfigMap with the manifests and operator info (openshift/assisted-service#7818) and in this PR the assisted-installer will retrieve the ConfigMap and apply the manifests.

The intent is that this will only be done for the agent-based installer and not affect the normal path of applying manifests using the assisted-service API.

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.

@bfournie
Copy link
Author

bfournie commented Jul 9, 2025

/cc @andfasano

@openshift-ci openshift-ci bot requested a review from andfasano July 9, 2025 14:38
@openshift-ci openshift-ci bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 9, 2025
@openshift-ci openshift-ci bot requested review from rccrdpccl and romfreiman July 9, 2025 14:43
Copy link

openshift-ci bot commented Jul 9, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: bfournie
Once this PR has been reviewed and has the lgtm label, please assign danielerez for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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

Copy link

codecov bot commented Jul 9, 2025

Codecov Report

❌ Patch coverage is 59.09091% with 27 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.06%. Comparing base (5ca92bd) to head (68675d1).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
...taller_controller/assisted_installer_controller.go 68.51% 12 Missing and 5 partials ⚠️
...ed-installer-controller/assisted_installer_main.go 0.00% 10 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1175      +/-   ##
==========================================
+ Coverage   55.71%   56.06%   +0.35%     
==========================================
  Files          15       15              
  Lines        3450     3510      +60     
==========================================
+ Hits         1922     1968      +46     
- Misses       1323     1330       +7     
- Partials      205      212       +7     
Files with missing lines Coverage Δ
.../assisted_installer_controller/reboots_notifier.go 87.93% <100.00%> (ø)
...ed-installer-controller/assisted_installer_main.go 26.28% <0.00%> (-1.26%) ⬇️
...taller_controller/assisted_installer_controller.go 76.59% <68.51%> (+0.70%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bfournie bfournie force-pushed the agent-operators-configmap branch 3 times, most recently from 1085219 to bfa1a01 Compare July 12, 2025 00:03
Content string
}

// Contents of ConfigMap containing operator manifests
type operatorMetadata struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

The internal directory where this is defined can't be vendored and it didn't seem worth it to create a new model file just for it.

}
}
// Apply manifests from configMap if ready
if !c.applyOperatorManifests(olmOperators, manifests, kubeconfigPath) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I was expecting this method to just retrieve the manifests (instead of querying them to AS)? Wasn't the PostInstallConfigs already responsible for applying them?

Copy link
Author

@bfournie bfournie Jul 30, 2025

Choose a reason for hiding this comment

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

PostInstallConfigs() isn't called since it uses the API, which is why this function - PostInstallConfigsK8sClient() was added to only use the Kubernetes API to apply the manifests. This is the only place they will be applied.

See https://github.com/openshift/assisted-installer/pull/1175/files#diff-887250d2b95a0f306e2f0c1d1f04628837775eb646462b4456d412768ea8632aR207-R211

@bfournie bfournie force-pushed the agent-operators-configmap branch 2 times, most recently from b853750 to 1cd1aa5 Compare July 31, 2025 13:02
In the agent-based installer, when cluster installation is complete the
assisted-installer cannot access assisted-service since it has
terminated. Therefore any operator custom-manifests are not applied.
Instead the assisted-service will create a ConfigMap with the
manifests and operator info (openshift/assisted-service#7818)
and in this PR the assisted-installer will retrieve the ConfigMap and
apply the manifests.

The intent is that this will only be done for the agent-based installer
and not affect the normal path of applying manifests using the
assisted-service API.
@bfournie bfournie force-pushed the agent-operators-configmap branch from 1cd1aa5 to 68675d1 Compare July 31, 2025 13:58
Copy link

openshift-ci bot commented Jul 31, 2025

@bfournie: all tests passed!

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-sigs/prow repository. I understand the commands that are listed here.

@rccrdpccl rccrdpccl removed their request for review August 28, 2025 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants