Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verify Native APIs Present for ClusterServiceVersion #541

Merged
merged 1 commit into from
Nov 2, 2018

Conversation

alecmerdler
Copy link
Member

Description

Adds the ability to enforce that certain "native" APIs exist in a Kubernetes cluster in order to install an Operator using a ClusterServiceVersion. This is a more complete solution than depending on some "version" of Kubernetes, because there are so many flavors and distributions that may or may not contain the APIs consumed by the Operator.

Changes

  • Adds spec.nativeAPIs field to ClusterServiceVersion (array of GroupVersionKinds)
  • Verifies required native APIs are present using basic discovery as part of CSV control loop
  • Writes out to requirementStatus just like CRDs and apiservices

Addresses https://jira.coreos.com/browse/ALM-666

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: alecmerdler

If they are not already assigned, you can assign the PR to them by writing /assign @alecmerdler in a comment when ready.

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 26, 2018
// that can manage apps for given version and AppType.
// NativeAPI describes the GVK of a Kubernetes resource that is required by an
// Operator, but must be provided by the underlying cluster and not an extension.
type NativeAPI struct {
Copy link
Member Author

Choose a reason for hiding this comment

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

There might be an existing struct that can be used here that is JSON-serializable.

Copy link

Choose a reason for hiding this comment

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

@ecordell
Copy link
Member

Looks great :) just need an e2e test and we can merge

@alecmerdler alecmerdler force-pushed the ALM-666 branch 2 times, most recently from d927772 to 0814f13 Compare October 29, 2018 18:03
@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 29, 2018
@@ -180,7 +180,25 @@ spec:
values:
type: array
description: set of values for the expression

nativeAPIs:
Copy link
Member Author

Choose a reason for hiding this comment

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

We can't enforce this as a required field, but it should be.

@alecmerdler alecmerdler force-pushed the ALM-666 branch 4 times, most recently from 276c901 to 2cacdfb Compare October 30, 2018 19:02
@@ -238,7 +260,7 @@ func (a *Operator) requirementAndPermissionStatus(csv *v1alpha1.ClusterServiceVe
return false, nil, fmt.Errorf("could not cast install strategy as type %T", strategyDetailsDeployment)
}

reqMet, reqStatuses := a.requirementStatus(strategyDetailsDeployment, csv.GetAllCRDDescriptions(), csv.GetOwnedAPIServiceDescriptions(), csv.GetRequiredAPIServiceDescriptions())
reqMet, reqStatuses := a.requirementStatus(strategyDetailsDeployment, csv.GetAllCRDDescriptions(), csv.GetOwnedAPIServiceDescriptions(), csv.GetRequiredAPIServiceDescriptions(), csv.Spec.NativeAPIs)

Copy link

Choose a reason for hiding this comment

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

Does the GUI ever make use of the previous getter methods? I'm just wondering if it's worth making one for the Native APIs to return an alphabetical listing (or deduplication) like the others.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't understand how the frontend app would call into the Operator Go source...?

Copy link

Choose a reason for hiding this comment

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

I didn't mean call directly. But based on your response there's zero coupling.

@alecmerdler
Copy link
Member Author

alecmerdler commented Nov 2, 2018

@ecordell Looks like this passed CI!

@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 2, 2018
@alecmerdler alecmerdler merged commit 9af7657 into operator-framework:master Nov 2, 2018
@alecmerdler alecmerdler deleted the ALM-666 branch November 2, 2018 18:32
ecordell pushed a commit to ecordell/operator-lifecycle-manager that referenced this pull request Mar 8, 2019
Verify Native APIs Present for ClusterServiceVersion
@njhale njhale added the kind/feature Categorizes issue or PR as related to a new feature. label Mar 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants