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

feat: revert #802 and use experimental to control the returned filtered version #816

Merged
merged 3 commits into from
Aug 24, 2022

Conversation

dduportal
Copy link
Contributor

@dduportal dduportal commented Aug 22, 2022

Requires #815
Related to #803
Reverts #802

Closes #810

This PR reverts the behavior of #802 by default, but allow it to be used when the new --experimental from #815 is used.

It also adds a warning to end user when the parsed version is different than the original, which means there is v prefix that was dropped.

The warning message (see below):

  • Gives instructions to end user on how to edit their manifest if they need to keep the "dropping v prefix" and how to test it (by adding the --experimental flag)
  • Is not shown if the experimental flag is enabled

Test

To test this pull request, you can run the following commands:

$ go build -o dist/updatecli

Then

$ ./dist/updatecli diff --config ./updatecli/updatecli.d/venom.yaml

./dist/updatecli diff --config ./updatecli/updatecli.d/venom.yaml


+++++++++++
+ PREPARE +
+++++++++++

Loading Pipeline "./updatecli/updatecli.d/venom.yaml"

SCM repository retrieved: 1


++++++++++++
+ PIPELINE +
++++++++++++



######################
# BUMP VENOM VERSION #
######################


SOURCES
=======

latestVersion
-------------
Searching for version matching pattern "*"
WARNING: Updatecli will soon stop removing the 'v' prefix of 'semver' version filters to keep the original retrieved version as per https://github.com/updatecli/updatecli/issues/803.
  If you need to keep the old behavior, please add a transformer (https://www.updatecli.io/docs/core/transformer/) of type "trimprefix" to remove the "v" prefix:

  sources:
    latestVersion:
      name: Get latest version
      kind: githubrelease
      spec:
        # ...
        versionfilter:
          kind: semver
      transformers:
        - trimprefix: 'v'

  You can try the new behavior (to verify your updated manifests) by adding the flag "--experimental" when executing the "updatecli" command line.
✔ Github Release version "1.0.1" found matching pattern "*"


CHANGELOG:
----------

Release published on the 2021-12-13 16:52:12 +0000 UTC at the url https://github.com/ovh/venom/releases/tag/v1.0.1

# Venom v1.0.1

## What's Changed
* fix: handle json number is assertions (#460) https://github.com/ovh/venom/pull/464

**Full Changelog**: https://github.com/ovh/venom/compare/v1.0.0...v1.0.1


TARGETS
========

goWorkflow
----------

**Dry Run enabled**

⚠ updated the [dry run] content of the file "/var/folders/2s/09szbrgn22l2tcvxz1_k34g40000gn/T/updatecli/github/updatecli/updatecli/.github/workflows/go.yaml"

--- /var/folders/2s/09szbrgn22l2tcvxz1_k34g40000gn/T/updatecli/github/updatecli/updatecli/.github/workflows/go.yaml
+++ /var/folders/2s/09szbrgn22l2tcvxz1_k34g40000gn/T/updatecli/github/updatecli/updatecli/.github/workflows/go.yaml
@@ -29,7 +29,7 @@
           sudo chmod +x /usr/local/bin/venom
           ls -lha /usr/local/bin/venom
         env:
-          VENOM_VERSION: v1.0.1
+          VENOM_VERSION: 1.0.1
       - name: Show Venom version
         run: venom version
       - name: Install GoReleaser



PULL REQUESTS
=============

[Dry Run] A pull request with kind "github" and title "[updatecli] Bump Venom version to 1.0.1" is expected.

=============================

REPORTS:


⚠ VENOM.YAML:
        Source:
                ✔ [latestVersion] Get latest Venom release (kind: githubrelease)
        Target:
                ⚠ [goWorkflow] Bump Venom version to 1.0.1 (kind: file)


Run Summary
===========
Pipeline(s) run:
  * Changed:    1
  * Failed:     0
  * Skipped:    0
  * Succeeded:  0
  * Total:      1
====

./dist/updatecli --experimental diff --config ./updatecli/updatecli.d/venom.yaml
Experimental Mode Enabled


+++++++++++
+ PREPARE +
+++++++++++

Loading Pipeline "./updatecli/updatecli.d/venom.yaml"

SCM repository retrieved: 1


++++++++++++
+ PIPELINE +
++++++++++++



######################
# BUMP VENOM VERSION #
######################


SOURCES
=======

latestVersion
-------------
Searching for version matching pattern "*"
✔ Github Release version "v1.0.1" found matching pattern "*"


CHANGELOG:
----------

Release published on the 2021-12-13 16:52:12 +0000 UTC at the url https://github.com/ovh/venom/releases/tag/v1.0.1

# Venom v1.0.1

## What's Changed
* fix: handle json number is assertions (#460) https://github.com/ovh/venom/pull/464

**Full Changelog**: https://github.com/ovh/venom/compare/v1.0.0...v1.0.1


TARGETS
========

goWorkflow
----------

**Dry Run enabled**

✔ Content from file "/var/folders/2s/09szbrgn22l2tcvxz1_k34g40000gn/T/updatecli/github/updatecli/updatecli/.github/workflows/go.yaml" already up to date
✔ All contents from 'file' and 'files' combined already up to date


PULL REQUESTS
=============


=============================

REPORTS:


✔ VENOM.YAML:
        Source:
                ✔ [latestVersion] Get latest Venom release (kind: githubrelease)
        Target:
                ✔ [goWorkflow] Bump Venom version to v1.0.1 (kind: file)


Run Summary
===========
Pipeline(s) run:
  * Changed:    0
  * Failed:     0
  * Skipped:    0
  * Succeeded:  1
  * Total:      1

Additional Information

Tradeoff

Potential improvement

olblak
olblak previously approved these changes Aug 22, 2022
@olblak
Copy link
Member

olblak commented Aug 22, 2022

Thanks for the quick pullrequest

@olblak
Copy link
Member

olblak commented Aug 23, 2022

I don't understand why tests are taking so long on this PR :/

@olblak olblak enabled auto-merge (squash) August 23, 2022 06:15
@dduportal
Copy link
Contributor Author

If you can wait 2 hours, i’ll rebase and check

…ed filtered version

Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
@dduportal
Copy link
Contributor Author

I don't understand why tests are taking so long on this PR :/

Rebase. But still the tests timeout for the end-to-end. I'm trying locally if I can make the venom setup working again (last time I checked it was a nightmare outside Linux)

@olblak
Copy link
Member

olblak commented Aug 23, 2022

I can easily check locally

@dduportal
Copy link
Contributor Author

I can easily check locally

I'm 99% that I forgot to update an end to end test: I bet for the one that (as I remember) checks for "no warnings" means that the manifest has to be updated or removed.

@olblak
Copy link
Member

olblak commented Aug 23, 2022

I found what's wrong and to be honest I don't know how to fix it :/
It has something to do with the dockerimage source

@olblak
Copy link
Member

olblak commented Aug 23, 2022

Basically the parsedVersion doesn't work with dockerimage, hence the discussion that triggered this work :)

The reason for that is because of the way dockerimage source works.
We need to do two api calls.

.1 Get tags
The first api call retrieves the list of image tags, which doesn't contain architecture information unfortunately.We apply the versionFilter to that list of tags to get the tag that match our need such as the latest semantic version.

.2 Validate tag architecture
then a second api call to validate that the tag match our requirement such as architecture. If something goes wrong like the wrong architecture, then we remove that tag from the list and check the next tag in the list.

Problem:
The problem in the e2e test, is that list of tags for ghcr.io/updatecli/updatecli has >170 tag they are all have the prefix "v"
So instead of getting tag manifest for ghcr.io/updatecli/updatecli:v0.29.0 we try ghcr.io/updatecli/updatecli:0.29.0 which obviously fail so we look for the next one v0.28.0, etc.

Because the parsedVersion

@olblak
Copy link
Member

olblak commented Aug 23, 2022

This is why I was limiting the backward compatibility to githubrelease and gittag

@olblak
Copy link
Member

olblak commented Aug 23, 2022

The failing manifest is :

./bin/updatecli diff --config e2e/updatecli.d/warning.d/dockerimage.yaml

@dduportal
Copy link
Contributor Author

Does it work if you add the —experimental flag in the e2e ?

@olblak
Copy link
Member

olblak commented Aug 23, 2022

Yes it does

@dduportal
Copy link
Contributor Author

Yes it does

Then i have to update the e2e test for this case as part of this pr

@olblak
Copy link
Member

olblak commented Aug 24, 2022

Yes it does

Then i have to update the e2e test for this case as part of this pr

What do you think to comment them out? I'll probably remove this from experimental in or right after the autodiscovery feature

@dduportal
Copy link
Contributor Author

Make sense!

Signed-off-by: Olblak <me@olblak.com>
@olblak
Copy link
Member

olblak commented Aug 24, 2022

If tests pass, then I'll merge and trigger a release :) thanks for the feedbacks

olblak
olblak previously approved these changes Aug 24, 2022
Signed-off-by: Olblak <me@olblak.com>
@olblak olblak added the experimental Experimental feaure label Aug 24, 2022
@olblak olblak removed the request for review from lemeurherve August 24, 2022 12:37
@olblak olblak merged commit 42fd5ce into updatecli:main Aug 24, 2022
@dduportal dduportal deleted the revert/802-version-filter-v-prefix branch August 24, 2022 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request experimental Experimental feaure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants