MGMT-4668: Generate OLM opeartor manifest to custom file #1609

machacekondra · 2021-04-28T14:33:31Z

This PR add a possibility to create manifests from assisted-installer.
In some cases it may be useful to create manifest from
assisted-isntaller instead of openshift-installer. For example when
installing CNV/LSO/OCS we need to have predefined CRDs for the
operators, because at the time of the operator is being created the CRDs
are not yet created by the OLM.

This PR add support to add the files to the bootstrap.ign to directory
prefix /opt/openshift/olmoperators, so later assisted-installer can fetch
the content of those files and create them via dynamic client.

Signed-off-by: Ondra Machacek omachace@redhat.com

openshift-ci-robot · 2021-04-28T14:33:49Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: machacekondra
To complete the pull request process, please assign yuvigold after the PR has been reviewed.
You can assign the PR to them by writing /assign @yuvigold in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

machacekondra · 2021-04-28T14:34:08Z

Related-to: openshift/assisted-installer#271

pkliczewski · 2021-05-04T10:07:22Z

@eranco74 @tsorya please review

tsorya · 2021-05-04T12:51:54Z

@machacekondra why it is different than creating manifests with bootkube by adding them to manifests? Trying to understand when we expect those manifests to be created? after cluster is up and running?

machacekondra · 2021-05-04T13:01:55Z

@machacekondra why it is different than creating manifests with bootkube by adding them to manifests? Trying to understand when we expect those manifests to be created? after cluster is up and running?

The problem is that the operator CRDs are not created at the time the openshift-installer creates the manifests resources. That's why we had to store the CRD spec in assisted-service. If we would not create the CRD by ourselfs, the openshift-installer would simply fail to create the operator resources, because it's not yet created by OLM.

With this PR, we create only OLM's resources by the openshift-installer manifests and we create our own custom assisted-installer manifests, which are created after the openshift is ready, which means OLM is ready and the OLM CRDs are created and we can without any issue create our operators CRs.

tsorya · 2021-05-04T14:02:07Z

@machacekondra wondering if those manifests can't be applied from bootstrap. When control plane will be up, operators should start already no? cluster bootstrap should eventually succeed no?

eranco74 · 2021-05-04T15:02:52Z

The problem is that the operator CRDs are not created at the time the openshift-installer
The CRDs will get created by the relevant operator once the operator will start running, no?

How would you do it if you were installing with openshift installer?

With this PR, we create only OLM's resources by the openshift-installer manifests and we create our own custom assisted-installer manifests, which are created after the openshift is ready

Who will apply these custom assisted-installer manifests?

tsorya · 2021-05-04T15:11:39Z

After talking with @eranco74 we think that this case is relevant only for SNO, regular flow should work without this one.
The main question what we are trying to solve here. SNO flow?
Another thing is that if we want it in SNO flow we should create api (if we don't have one already) that will allow to download those manifests and not to push them to bootstrap ignition as they shouldn't be part of it at all.

machacekondra · 2021-05-05T05:31:13Z

The problem is that the operator CRDs are not created at the time the openshift-installer
The CRDs will get created by the relevant operator once the operator will start running, no?

How would you do it if you were installing with openshift installer?

With this PR, we create only OLM's resources by the openshift-installer manifests and we create our own custom assisted-installer manifests, which are created after the openshift is ready

Who will apply these custom assisted-installer manifests?

assisted-installer

@machacekondra wondering if those manifests can't be applied from bootstrap. When control plane will be up, operators should start already no? cluster bootstrap should eventually succeed no?

I am not sure about this one. We wait for for the operators to be ready in assisted-installer posinstall stage so I think there could be some catch.

Another thing is that if we want it in SNO flow we should create api (if we don't have one already) that will allow to download those manifests and not to push them to bootstrap ignition as they shouldn't be part of it at all.

Why do you think it's SNO issue only? It's problem with normal installation as well. But I agree that creating a special API for those files would make a better sense. Something like:

/clusters/{cluster_id}/downloads/files?file_name=custom_manifests.yaml

And the content would be all the relevant custom manifests?

eranco74 · 2021-05-05T10:37:09Z

Pretty sure that it's relevant only for SNO because assisted-installer is capable to install OLM operators without creating the manifests from the assisted-controller.
It works the same when installing a cluster with openshift-installer, cluster-bootstrap will keep on trying to apply the manifests until it succeeds, at some point the relevant operator (that is creating the required CRDs for applying the manifest) will start running on the master nodes, create the CRDs and cluster bootstrap will succeed.
When installing SNO it's a different story (because there is no master node running in parallel with the bootstrap).
This is documented in the SNO bootstrap in place enhancement.
If this is what this code is trying to solve ( I don't think we want it for multi-node cluster in order to stay inline with openshift-installer behavior), we should decide which manifests get this special treatment, I guess only OLM related manifests that assisted-service support for SNO.

Also I agree there should be a dedicated API for these manifests, overloading them on the bootstrap ignition (bootstrap node never use them) and pulling the bootstrap ignition from the assisted-controller seems wrong.

machacekondra · 2021-05-06T15:44:09Z

So I've update the code to upload the manifest to storage. Example:

$ curl http://10.97.140.59:8090/api/assisted-install/v1/clusters/dcf20f64-fe45-404a-af45-a205856e27e8/downloads/files?file_name=custom_manifests.yaml
---
apiVersion: hco.kubevirt.io/v1beta1
kind: HyperConverged
metadata:
  name: kubevirt-hyperconverged
  namespace: openshift-cnv
spec:
  BareMetalPlatform: true
---
apiVersion: "local.storage.openshift.io/v1alpha1"
kind: "LocalVolumeSet"
metadata:
  name: "local-disks"
  namespace: "openshift-local-storage"
spec:
  storageClassName: "localblock-sc"
  volumeMode: Block
  deviceInclusionSpec:
    deviceTypes:
      - "disk"

machacekondra · 2021-05-06T17:24:20Z

/test subsystem-aws

machacekondra · 2021-05-17T06:29:17Z

/retest

machacekondra · 2021-05-17T15:00:15Z

/retest

machacekondra · 2021-05-18T05:45:33Z

/retest

machacekondra · 2021-05-19T06:52:57Z

@eranco74 @tsorya Can you please re-review?

tsorya · 2021-05-19T15:51:52Z

internal/operators/manager.go

 		}
 	}

 	return nil
 }

-func (mgr *Manager) createManifests(ctx context.Context, cluster *common.Cluster, filename string, content []byte) error {
+func (mgr *Manager) createCustomManifest(ctx context.Context, cluster *common.Cluster, content string) error {
+	objectFileName := fmt.Sprintf("%s/custom_manifests.yaml", cluster.ID)


createCustomManifest const it

eranco74

Looks good

eranco74 · 2021-05-20T08:00:33Z

internal/operators/manager.go

 		}
 	}

 	return nil
 }

-func (mgr *Manager) createManifests(ctx context.Context, cluster *common.Cluster, filename string, content []byte) error {
+func (mgr *Manager) createCustomManifest(ctx context.Context, cluster *common.Cluster, content string) error {


Consider adding a comment explaining that this is for custom manifests that requires CRDs are created late by the cluster operators (and not by the CVO)

eranco74 · 2021-05-20T08:12:38Z

/lgtm

This PR add a possibility to create manifests from assisted-installer. In some cases it may be useful to create manifest from assisted-isntaller instead of openshift-installer. For example when installing CNV/LSO/OCS we need to have predefined CRDs for the operators, because at the time of the operator is being created the CRDs are not yet created by the OLM. This PR add support to read the files from custom_manifest.yaml and fetch the content of those files and create them via dynamic client. Since assisted-installer-controller runs with limited permissions, every resource which should be created by assisted_installer_controller must be added to the clusterrole. Signed-off-by: Ondra Machacek <omachace@redhat.com>

openshift-ci · 2021-05-21T08:24:36Z

@machacekondra: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/e2e-metal-assisted-operator-disconnected	`061dfbf`	link	`/test e2e-metal-assisted-operator-disconnected`

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.

machacekondra · 2021-05-21T12:40:39Z

/retest

tsorya · 2021-05-23T08:01:11Z

/lgtm
/approve

openshift-ci · 2021-05-23T08:01:26Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: machacekondra, tsorya

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:

~~OWNERS~~ [tsorya]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

…on (openshift#1609)" This reverts commit c1fcc80.

pkliczewski · 2021-06-17T12:08:44Z

/cherry-pick ocm-2.3

openshift-cherrypick-robot · 2021-06-17T12:09:20Z

@pkliczewski: new pull request could not be created: failed to create pull request against openshift/assisted-service#ocm-2.3 from head openshift-cherrypick-robot:cherry-pick-1609-to-ocm-2.3: status code 422 not one of [201], body: {"message":"Validation Failed","errors":[{"resource":"PullRequest","code":"custom","message":"No commits between openshift:ocm-2.3 and openshift-cherrypick-robot:cherry-pick-1609-to-ocm-2.3"}],"documentation_url":"https://docs.github.com/rest/reference/pulls#create-a-pull-request"}

In response to this:

/cherry-pick ocm-2.3

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.

pkliczewski · 2021-06-17T12:16:09Z

This commit is already part of ocm-2.3

openshift-ci-robot requested review from pkliczewski and RazRegev April 28, 2021 14:33

machacekondra mentioned this pull request Apr 28, 2021

MGMT-4668: Add support to create manifest in assisted-installer openshift/assisted-installer#271

Merged

openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 28, 2021

machacekondra force-pushed the dont_use_crd branch from 8d553b0 to c76c92b Compare April 29, 2021 12:02

openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 29, 2021

machacekondra force-pushed the dont_use_crd branch from c76c92b to d4cddae Compare April 29, 2021 12:16

machacekondra force-pushed the dont_use_crd branch from d4cddae to bea6640 Compare May 6, 2021 15:43

openshift-ci bot added the api-review Categorizes an issue or PR as actively needing an API review. label May 6, 2021

machacekondra force-pushed the dont_use_crd branch from bea6640 to 061dfbf Compare May 7, 2021 06:25

machacekondra changed the title ~~MGMT-4668: Generate OLM opeartor manifest to bootstrap ignition~~ MGMT-4668: Generate OLM opeartor manifest to custom file May 10, 2021

machacekondra force-pushed the dont_use_crd branch from 061dfbf to 188385f Compare May 13, 2021 14:25

machacekondra force-pushed the dont_use_crd branch 3 times, most recently from 4b676e2 to 7810b0a Compare May 17, 2021 12:45

tsorya reviewed May 19, 2021

View reviewed changes

eranco74 reviewed May 20, 2021

View reviewed changes

openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 20, 2021

openshift-ci bot assigned eranco74 May 20, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 20, 2021

machacekondra force-pushed the dont_use_crd branch from 7810b0a to bd7bd2e Compare May 21, 2021 06:44

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label May 21, 2021

machacekondra force-pushed the dont_use_crd branch from bd7bd2e to 4cee2f3 Compare May 21, 2021 06:52

openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 21, 2021

openshift-ci bot assigned tsorya May 23, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 23, 2021

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 23, 2021

openshift-merge-robot merged commit c1fcc80 into openshift:master May 23, 2021

machacekondra added a commit to machacekondra/assisted-service that referenced this pull request Jun 1, 2021

Revert "MGMT-4668: Generate OLM opeartor manifest to bootstrap igniti…

add34e9

…on (openshift#1609)" This reverts commit c1fcc80.

machacekondra mentioned this pull request Jun 1, 2021

Revert "MGMT-4668: Generate OLM opeartor manifest to bootstrap igniti… #1876

Closed

machacekondra added a commit to machacekondra/assisted-service that referenced this pull request Jun 1, 2021

Revert "MGMT-4668: Generate OLM opeartor manifest to bootstrap igniti…

66d5ab6

…on (openshift#1609)" This reverts commit c1fcc80.

machacekondra added a commit to machacekondra/assisted-service that referenced this pull request Jun 1, 2021

Revert "MGMT-4668: Generate OLM opeartor manifest to bootstrap igniti…

05ad273

…on (openshift#1609)" This reverts commit c1fcc80.

machacekondra mentioned this pull request Jun 1, 2021

Revert "MGMT-4668: Generate OLM opeartor manifest to bootstrap igniti #1877

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MGMT-4668: Generate OLM opeartor manifest to custom file #1609

MGMT-4668: Generate OLM opeartor manifest to custom file #1609

machacekondra commented Apr 28, 2021 •

edited

openshift-ci-robot commented Apr 28, 2021

machacekondra commented Apr 28, 2021

pkliczewski commented May 4, 2021

tsorya commented May 4, 2021

machacekondra commented May 4, 2021

tsorya commented May 4, 2021

eranco74 commented May 4, 2021

tsorya commented May 4, 2021 •

edited

machacekondra commented May 5, 2021

eranco74 commented May 5, 2021

machacekondra commented May 6, 2021

machacekondra commented May 6, 2021

machacekondra commented May 17, 2021

machacekondra commented May 17, 2021

machacekondra commented May 18, 2021

machacekondra commented May 19, 2021

tsorya May 19, 2021

eranco74 left a comment

eranco74 May 20, 2021

eranco74 commented May 20, 2021

openshift-ci bot commented May 21, 2021 •

edited

machacekondra commented May 21, 2021

tsorya commented May 23, 2021

openshift-ci bot commented May 23, 2021

pkliczewski commented Jun 17, 2021

openshift-cherrypick-robot commented Jun 17, 2021

pkliczewski commented Jun 17, 2021

MGMT-4668: Generate OLM opeartor manifest to custom file #1609

MGMT-4668: Generate OLM opeartor manifest to custom file #1609

Conversation

machacekondra commented Apr 28, 2021 • edited

openshift-ci-robot commented Apr 28, 2021

machacekondra commented Apr 28, 2021

pkliczewski commented May 4, 2021

tsorya commented May 4, 2021

machacekondra commented May 4, 2021

tsorya commented May 4, 2021

eranco74 commented May 4, 2021

tsorya commented May 4, 2021 • edited

machacekondra commented May 5, 2021

eranco74 commented May 5, 2021

machacekondra commented May 6, 2021

machacekondra commented May 6, 2021

machacekondra commented May 17, 2021

machacekondra commented May 17, 2021

machacekondra commented May 18, 2021

machacekondra commented May 19, 2021

tsorya May 19, 2021

Choose a reason for hiding this comment

eranco74 left a comment

Choose a reason for hiding this comment

eranco74 May 20, 2021

Choose a reason for hiding this comment

eranco74 commented May 20, 2021

openshift-ci bot commented May 21, 2021 • edited

machacekondra commented May 21, 2021

tsorya commented May 23, 2021

openshift-ci bot commented May 23, 2021

pkliczewski commented Jun 17, 2021

openshift-cherrypick-robot commented Jun 17, 2021

pkliczewski commented Jun 17, 2021

machacekondra commented Apr 28, 2021 •

edited

tsorya commented May 4, 2021 •

edited

openshift-ci bot commented May 21, 2021 •

edited