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

Fix/operator form options #869

Conversation

mateusoliveira43
Copy link
Collaborator

@mateusoliveira43 mateusoliveira43 commented Mar 7, 2023

Describe the behavior changes introduced in this PR

Allow form view to gave all fields user can have, when creating instance from Pelorus Operator.

Linked Issues

Resolves #860

Testing Instructions

Use Pelorus operator from mine quay repository

oc login ...
oc delete namespace pelorus
oc create namespace pelorus
operator-sdk run bundle quay.io/msouzaol/pelorus-operator-bundle:v0.0.6 --namespace pelorus

Then, create an instance of Pelorus using form or yaml view, to test if it is working. It should warn of wrong usage or lack of required fields.

To clean it up, run

operator-sdk cleanup pelorus-operator --namespace pelorus

Signed-off-by: Mateus Oliveira <msouzaol@redhat.com>
Signed-off-by: Mateus Oliveira <msouzaol@redhat.com>
@mpryc
Copy link
Collaborator

mpryc commented Mar 8, 2023

Can the options be better grouped ? Also we need to discuss the layout before implementing them in the operator itself. Thinking here of options in the format of:

  Exporters: >
     Instances: >
        Exporter Name:
        Exporter Type:
        Secrets:
            + Add Secret name
        Config Maps
            + Add Config Map
        Extra config vars:
            + Add configuration option
        
        Development options: >
            Source Ref:
            Source URL:
            Image Name:
            Image Tag:

  Prometheus configuration:
    Internal Prometheus Htpasswd
    Openshift Prometheus Basic Auth Pass

  Prometheus Storage:
     prometheus_storage: >(expand)
        Prometheus Retention
        Prometheus Retention Size
        Prometheus Storage Pvc Capacity
        Prometheus Storage Pvc Storageclass

  Thanos s3 storage: >(expand)
    Bucket Name
    Bucket Access Point
    Bucket Access Key
    Bucket Secret Access Key
    Extra: > (expand)
      Thanos Version

@mateusoliveira43
Copy link
Collaborator Author

mateusoliveira43 commented Mar 8, 2023

I do not know @mpryc , I will try out

Signed-off-by: Mateus Oliveira <msouzaol@redhat.com>
Signed-off-by: Mateus Oliveira <msouzaol@redhat.com>
Signed-off-by: Mateus Oliveira <msouzaol@redhat.com>
@mateusoliveira43
Copy link
Collaborator Author

I think we can not group them, but reordered them @mpryc

@mpryc
Copy link
Collaborator

mpryc commented Mar 9, 2023

How about the following top structure (and it's top-level titles):

- path: exporters                              | Exporters
- path: external_prometheus_hosts              | External Exporters
- path: prometheus_retention                   | Prometheus Retention
- path: prometheus_retention_size              | Prometheus Retention Size
- path: prometheus_storage                     | Prometheus Persistent Storage (PVC) 
- path: prometheus_storage_pvc_capacity        | Prometheus PVC Capacity
- path: prometheus_storage_pvc_storageclass    | Prometheus PVC Storageclass
- path: federated_prometheus_hosts             | Prometheus Federated Hosts
- path: openshift_prometheus_htpasswd_auth     | Prometheus Internal Auth (htpasswd)
- path: openshift_prometheus_basic_auth_pass   | Prometheus Internal Auth (basic auth)
- path: thanos_bucket_name                     | Thanos S3 Bucket Name
- path: bucket_access_point                    | Thanos S3 Access Point
- path: bucket_access_key                      | Thanos S3 Access Key
- path: bucket_secret_access_key               | Thanos S3 Secret Access Key
- path: thanos_version                         | Thanos Quay Image Tag
- path: custom_ca                              | CA Certificate Injection

@mpryc
Copy link
Collaborator

mpryc commented Mar 9, 2023

The path can be used with dots, can this be used to also define exporters order?

https://github.com/openshift/console/blob/master/frontend/packages/operator-lifecycle-manager/src/components/descriptors/reference/reference.md#specdescriptors-1

@mpryc
Copy link
Collaborator

mpryc commented Mar 9, 2023

Can the secrets and configmaps be used similarly to this example? https://github.com/openshift/console/blob/master/frontend/packages/operator-lifecycle-manager/src/components/descriptors/reference/reference.md#3-k8sresourceprefix

Signed-off-by: Mateus Oliveira <msouzaol@redhat.com>
Signed-off-by: Mateus Oliveira <msouzaol@redhat.com>
@mateusoliveira43
Copy link
Collaborator Author

mateusoliveira43 commented Mar 9, 2023

I think I addressed everything now @mpryc

to test it, operator-sdk run bundle quay.io/msouzaol/pelorus-operator-bundle:v0.0.5 --namespace pelorus

Signed-off-by: Mateus Oliveira <msouzaol@redhat.com>
@openshift-ci
Copy link

openshift-ci bot commented Mar 9, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mpryc

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 openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 9, 2023
@mpryc
Copy link
Collaborator

mpryc commented Mar 9, 2023

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 9, 2023
@mateusoliveira43
Copy link
Collaborator Author

/retest

@openshift-merge-robot openshift-merge-robot merged commit 7ed22f5 into dora-metrics:master Mar 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Config options other then specified in the open api schema are not used
3 participants