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.

Sign up for GitHub

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

Jump to bottom

alm-examples isn't copied over to new CSV when using --from-version #1847

Closed
jpkrohling opened this issue Aug 21, 2019 · 7 comments
Closed

alm-examples isn't copied over to new CSV when using --from-version #1847

jpkrohling opened this issue Aug 21, 2019 · 7 comments
Assignees
@jmrodri @camilamacedo86
Labels
kind/bug Categorizes issue or PR as related to a bug. triage/support Indicates an issue that is a support question.

Comments

@jpkrohling
Copy link

jpkrohling commented Aug 21, 2019 •
edited

Bug Report

When generating a new CSV with a --from-version flag, the annotation alm-example isn't being copied over to the new CSV.

What did you do?

$ GO111MODULE=on operator-sdk olm-catalog gen-csv --csv-channel=stable --default-channel --csv-version=1.13.1
INFO[0000] Generating CSV manifest version 1.13.1       
INFO[0000] Fill in the following required fields in file deploy/olm-catalog/jaeger-operator/1.13.1/jaeger-operator.v1.13.1.clusterserviceversion.yaml:
	spec.keywords
	spec.maintainers
	spec.provider 
INFO[0000] Created deploy/olm-catalog/jaeger-operator/1.13.1/jaeger-operator.v1.13.1.clusterserviceversion.yaml 
INFO[0000] Created deploy/olm-catalog/jaeger-operator/jaeger-operator.package.yaml 


$ ## fix the jaeger-operator.package.yaml as per #1846 and replaced the contents with our [current CSV](https://github.com/jaegertracing/jaeger-operator/blob/9cdd514655654a92de5a70b58fda2797d6a6a32b/deploy/olm-catalog/jaeger.clusterserviceversion.yaml)

$ GO111MODULE=on operator-sdk olm-catalog gen-csv --csv-channel=stable --default-channel --csv-version=1.13.2 --from-version=1.13.1
INFO[0000] Generating CSV manifest version 1.13.2       
INFO[0000] Created deploy/olm-catalog/jaeger-operator/1.13.2/jaeger-operator.v1.13.2.clusterserviceversion.yaml 
INFO[0000] Created deploy/olm-catalog/jaeger-operator/jaeger-operator.package.yaml

What did you expect to see?

Before the last command, the alm-examples annotation from deploy/olm-catalog/jaeger-operator/1.13.1/jaeger-operator.v1.13.1.clusterserviceversion.yaml look like this:

    alm-examples: |-
      [
        {
          "apiVersion": "jaegertracing.io/v1",
          "kind": "Jaeger",
          "metadata": {
            "name": "my-jaeger"
          },
          "spec": {
            "strategy": "allInOne",
            "allInOne": {
              "image": "jaegertracing/all-in-one:1.13",
              "options": {
                "log-level": "debug",
                "query": {
                  "base-path": "/jaeger"
                }
              }
            },
            "ui": {
              "options": {
                "dependencies": {
                  "menuEnabled": false
                },
                "tracking": {
                  "gaID": "UA-000000-2"
                },
                "menu": [
                  {
                    "label": "About Jaeger",
                    "items": [
                      {
                        "label": "Documentation",
                        "url": "https://www.jaegertracing.io/docs/latest"
                      }
                    ]
                  }
                ]
              }
            },
            "storage": {
              "options": {
                "memory": {
                  "max-traces": 100000
                }
              }
            }
          }
        }
      ]

The above is what I'd expect to see in the CSV for 1.13.2, but this is what I see instead:

    alm-examples: '[{"apiVersion":"io.jaegertracing/v1alpha1","kind":"Jaeger","metadata":{"name":"example-jaeger"}},{"apiVersion":"jaegertracing.io/v1","kind":"Jaeger","metadata":{"name":"example-jaeger"}}]'

Environment

  • operator-sdk version: operator-sdk version: v0.10.0, commit: ff80b17737a6a0aade663e4827e8af3ab5a21170
  • go version: go version go1.12.5 linux/amd64
  • Kubernetes version information: not relevant, as those are "offline" operations
  • Are you writing your operator in ansible, helm, or go?

Additional context
Found while working on jaegertracing/jaeger-operator#604
This was referenced Aug 21, 2019
Closed
Merged
@jmrodri jmrodri self-assigned this Aug 26, 2019
@jmrodri
Copy link
Member

jmrodri commented Aug 26, 2019

Thanks for the bug report, I'll take a look at this one.

@jmrodri jmrodri added kind/bug Categorizes issue or PR as related to a bug. triage/support Indicates an issue that is a support question. labels Aug 26, 2019
@jpkrohling
Copy link
Author

Not trying to put pressure or anything, but for our own planning purposes: is this on the roadmap to be fixed?

@camilamacedo86
Copy link
Contributor

camilamacedo86 commented Sep 19, 2019 •
edited

Hi @jmrodri and @jpkrohling,

It is already implemented and merged into the master branch. See here. Please, could you check the master branch and let us know. Please, also let me know if I missing something here. Is not it working when a new version of the same CSV is generated, is this the issue with?

@jpkrohling
Copy link
Author

It does seem to work, however, the contents of alm-examples isn't copied over from the previous version (--from-version). Instead, it's populated based on the CRs available under deploy/crds.

If that's the expected behavior, then it looks good and this issue can be closed.

@camilamacedo86
Copy link
Contributor

camilamacedo86 commented Sep 19, 2019 •
edited

HI @jpkrohling,

The issue described here is over the examples are not be pretty then I understand that it is solved in the master already. Am I right?

Additionally to that, you are saying that when you change manually the examples these changes are NOT copied for a new version of CSV. So, IMHO it should NOT be copied at all, the user/dev should perform the changes in the CR in order to check changes into this section instead of doing it manually. HI, @joelanford, do not you agree? WDYT?

@jpkrohling
Copy link
Author

Sorry, I think this bug report wasn't even valid in the first place. I originally missed the fact that the alm-examples is populated from CRs at deploy/crds. The fact that they aren't pretty-printed didn't help either. As a result, I'm closing this issue.

My expectation at the beginning was that, when using --from-version=1.0.0, the whole CSV would be copied over to the new version, and only the version fields would be bumped. So, the alm-examples that I manually set for 1.0.0 would be used on 1.0.1.

@camilamacedo86
Copy link
Contributor

Hi @jpkrohling,

Really tks for your collab 🥇 .

If we do not use the data from the CR then the users will need to update the examples manually which IMHO is very harder and the CSV will no longer be aligned/synchronized with the code source. Because of this, I agree with you that it would not be a bug at all.

Thank you for your commitment to improving this project.

@camilamacedo86 camilamacedo86 self-assigned this Sep 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jmrodri jmrodri

@camilamacedo86 camilamacedo86

Labels
kind/bug Categorizes issue or PR as related to a bug. triage/support Indicates an issue that is a support question.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jpkrohling @jmrodri @camilamacedo86