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

Feature/upgrade operator-sdk 1.27.0 #1498

Merged
merged 14 commits into from
Mar 9, 2023
Merged

Feature/upgrade operator-sdk 1.27.0 #1498

merged 14 commits into from
Mar 9, 2023

Conversation

yuriolisa
Copy link
Contributor

@yuriolisa yuriolisa commented Feb 22, 2023 •
edited by pavolloffay
Loading

Resolves #1501

@yuriolisa yuriolisa requested a review from a team February 22, 2023 14:03
@yuriolisa yuriolisa added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Feb 22, 2023
@yuriolisa yuriolisa mentioned this pull request Feb 22, 2023
Closed
@iblancasa
Copy link
Contributor

Crossreference #1501.

Could you also update the title of the PR?

@iblancasa
Copy link
Contributor

Update the version in the scorecar tests

@yuriolisa yuriolisa changed the title Feature/upgrade operator sdk 124 Feature/upgrade operator-sdk 1.27 Feb 23, 2023
@yuriolisa yuriolisa changed the title Feature/upgrade operator-sdk 1.27 Feature/upgrade operator-sdk 1.27.0 Feb 23, 2023
@yuriolisa
Adding createdAt fix
0bf3628 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
@yuriolisa
Adding createdAt fix
1813a64 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
@yuriolisa
Copy link
Contributor Author

Update the version in the scorecar tests

It shouldn't be updated.

@pavolloffay
Copy link
Member

It shouldn't be updated.

@yuriolisa any reason why it should not be updated? It was always bumped as part of the SDK version bump. Also see the example operator https://github.com/operator-framework/operator-sdk/blob/master/testdata/go/v3/memcached-operator/bundle/tests/scorecard/config.yaml

@yuriolisa
Adding scorecard-test update
2c54a8a 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
pavolloffay
pavolloffay reviewed Feb 23, 2023
View reviewed changes
Copy link
Member

@pavolloffay pavolloffay left a comment

Choose a reason for hiding this comment

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

After the update the project structure and dependency stack should be aligned with https://github.com/operator-framework/operator-sdk/blob/master/testdata/go/v3/memcached-operator

Therefore:

  • CONTROLLER_TOOLS_VERSION should be bumped to 0.10.0

@yuriolisa
Copy link
Contributor Author

It shouldn't be updated.

@yuriolisa any reason why it should not be updated? It was always bumped as part of the SDK version bump. Also see the example operator https://github.com/operator-framework/operator-sdk/blob/master/testdata/go/v3/memcached-operator/bundle/tests/scorecard/config.yaml

Thank you for the heads-up. I've mislooked at the scorecard-test version.

@pavolloffay
Copy link
Member

@yuriolisa unit tests failed

@yuriolisa
Fixing tools version
49720c9 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
@yuriolisa
Fixing tools version
ace61d3 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
@yuriolisa
Fixing version
c82fc95 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
@yuriolisa
Fixing controller-gen version
e8c5d44 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
@yuriolisa
Fixing kubebuilder-assets
3ac6914 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
@yuriolisa
Fixing kubebuilder-assets
e8dbcf1 
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
frzifus
frzifus approved these changes Mar 2, 2023
View reviewed changes
bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml
@@ -31,9 +31,9 @@ metadata:
categories: Logging & Tracing
certified: "false"
containerImage: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator
createdAt: "2020-12-16T13:37:00+00:00"
createdAt: "2023-03-02T10:14:11Z"
Copy link
Member

Choose a reason for hiding this comment

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

hm.. is this information displayed somewhere? Ignoring it makes sense, otherwise our diff test would fail. But at the same time, this field will be 2023-03-02T10:14:11Z for ever.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFAIK, since the operator-sdk version 1.26, the createdAt field is being changed. Due to that, @iblancasa pointed to creating a workaround for it.

operator-framework/operator-sdk#6285

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems it was not updated until now. So... it seems it is not really important.

Copy link
Member

Choose a reason for hiding this comment

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

@yuriolisa yes, that thing helps to ignore the timestamp. But is it right to ignore this one?

  • If createdAt describes when the operator was first created, we should not update the timestamp.
  • If the field describes when the shipped version was created, we should update the field with each release.

(My understanding is that a field like createdAt should never change and instead a field like updatedAt should be changed with each release.)

On Operatorhub i found this:
image

Copy link
Contributor

Choose a reason for hiding this comment

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

Since @frzifus reacted with 👎🏻, I'll try to clarify my comment.

It seems it was not updated until now. So... it seems it is not really important.

By this I meant: if it was expected to be updated, nobody paid attention to it. Sorry if I was not clear enough.

I think we should not update it but since it seems it is not clear what it means... I don't have a strong opinion. It can be:

  • When the bundle was created
  • When the operator was created

Copy link
Member

Choose a reason for hiding this comment

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

A rough (to the day) timestamp of when the operator image was created

Sounds like we should update this field with every release.

https://redhat-connect.gitbook.io/certified-operator-guide/ocp-deployment/operator-metadata/creating-the-csv#fields-to-add-under-metadata.annotations

@yuriolisa
Copy link
Contributor Author

@pavolloffay , could you please have a look?

@pavolloffay pavolloffay merged commit b1b437f into open-telemetry:main Mar 9, 2023
@jaronoff97 jaronoff97 mentioned this pull request Nov 28, 2023
Closed
ItielOlenick pushed a commit to ItielOlenick/opentelemetry-operator that referenced this pull request May 1, 2024
@yuriolisa
Feature/upgrade operator-sdk 1.27.0 (open-telemetry#1498)
b33e539 
* Adding new PRs

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Upgrading Operator-sdk

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Adding createdAt fix

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Adding createdAt fix

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Adding scorecard-test update

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Fixing tools version

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Fixing version

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Fixing controller-gen version

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Fixing kubebuilder-assets

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

* Fixing kubebuilder-assets

Signed-off-by: Yuri Sa <yurimsa@gmail.com>

---------

Signed-off-by: Yuri Sa <yurimsa@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iblancasa iblancasa iblancasa approved these changes

@frzifus frzifus frzifus approved these changes

@pavolloffay pavolloffay pavolloffay approved these changes

Assignees
No one assigned
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Upgrade operator-sdk to 1.27.0
4 participants
@yuriolisa @iblancasa @pavolloffay @frzifus