Skip to content

Conversation

@jnweiger
Copy link

Not sure, where exactly this config.php is used -- maybe it is just for reference. That is also good to keep it updated.
Also pushed the same (I hope) to https://gitea.owncloud.services/devops/product/pulls/239

Please review (both) carefully.

Not sure, where exactly this config.php is used -- maybe it is just for reference. That is also good to keep it updated.
Also pushed the same (I hope) to https://gitea.owncloud.services/devops/product/pulls/239

Please review (both) carefully.
Copy link
Contributor

@phil-davis phil-davis left a comment

Choose a reason for hiding this comment

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

I adjusted the tests so that they have 10.9.0 and 10.9.1 and pass.

Comment on lines +84 to +91
'10.9' => [
'latest' => '10.9.1',
'web' => 'https://doc.owncloud.com/server/10.9/admin_manual/maintenance/upgrading/update.html',
],
'10.8' => [
'latest' => '10.9.1',
'web' => 'https://doc.owncloud.com/server/10.8/admin_manual/maintenance/upgrading/update.html',
],
Copy link
Contributor

Choose a reason for hiding this comment

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

why was this section previously empty?

Copy link
Member

Choose a reason for hiding this comment

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

@phil-davis because all channels were unified to deliver the same update path.
it was done by setting 'eol_latest' => '10.7.100', so 10.7.100 and below are using the same (EOL) channel.

Copy link
Member

@VicDeo VicDeo Jan 15, 2022

Choose a reason for hiding this comment

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

I mean they are using EOL channel disregarding the requested channel.
It allows to have more compact config - IRL all past releases use the same path (8-9-10-10.1-etc) and only one or two latest releases have differences in the different channels.
I hated to scroll a lot after changing one channel to reach another one. So I added an artificial channel that contains the common part of the past releases that is reused by all other channels.

@VicDeo
Copy link
Member

VicDeo commented Jan 15, 2022

This PR doesn't look complete. At least beta channel should be changed accordingly. And may be production.
Also new tests with expectations that no updates are NOT available for 10.9.1 should be added

Regarding the daily channel - 10.8 should be updated to 10.9.1 and 10.9.1 to daily zip

@VicDeo
Copy link
Member

VicDeo commented Jan 15, 2022

Regarding the usage:
This repo doesn't affect the REAL update server. I use it to add new tests and check whether all the tests are passing locally with make test

and then I push the similar changes to gitea. Once the PR at gitea is merged the changes are immediately deployed to the real server
and then it's possible to test real server using this repo with make test-production

Updated daily section as suggested by @VicDeo
@jnweiger
Copy link
Author

jnweiger commented Jan 19, 2022

At least beta channel should be changed accordingly. And may be production.
Also new tests with expectations that no updates are NOT available for 10.9.1 should be added

@VicDeo Can you provide some hints, how production and beta should look like?
No idea, how production is different from stable
I'd like to update "everything" to 10.9.1 but how do we code "everything"?
Can we simply copy the same data structure that we have in stable also to beta and production?

We should probably move 10.8.0 to the EOL list at some time in future... When?

I've updated daily in this PR. Is this what you meant?

Thank you very much, for all the other insights you provided here! Great help!

@jnweiger jnweiger requested a review from VicDeo January 19, 2022 17:35
@phil-davis
Copy link
Contributor

https://drone.owncloud.com/owncloud/owncloud-updater-server/56/2/2
3 test fail - they need looking at and adjusting, if the test expectations are now different.

@jnweiger
Copy link
Author

jnweiger commented Jan 20, 2022

https://drone.owncloud.com/owncloud/owncloud-updater-server/56/2/2 3 test fail - they need looking at and adjusting, if the test expectations are now different.

[Thu Jan 20 16:36:14 2022] 127.0.0.1:59464 Closing
    And Update to version "100.0.0.0" is available
    And URL to download is "https://download.owncloud.org/community/owncloud-daily-master.zip"
Expected url https://download.owncloud.org/community/owncloud-daily-master.zip does not equal https://download.owncloud.org/community/owncloud-10.9.1.zip (Exception)

When the version is returned as 100.0.0.0, then it should also return daily-master.zip -- But I don't see, where that 100.0.0.0 is coming from. I cannot say, weather test expectations need to adapt, or my settings are incorrect or both?

@phil-davis
Copy link
Contributor

phil-davis commented Jan 21, 2022

Note: I am not familiar with any of this - it is new to me. So don't be waiting for an informed reply from me.

Please can someone with knowledge answer here, otherwise I would be reverse-engineering this whole thing to know exactly what is supposed to happen.

@VicDeo
Copy link
Member

VicDeo commented Jan 21, 2022

@jnweiger version 100 is coming from the update server. Thus daily build could be updated to the same version if the date is newer. It's a daily channel-specific thing

regarding the channels https://owncloud.com/news/owncloud-release-channels/
I used to update production channel separately after all other channels are updated. Sometimes even before the next oC release comes into RC state.

regarding this PR - you updated the channels but some expectations are still not documented in tests.

@jnweiger
Copy link
Author

@phil-davis Do you know what needs to be done to bring the new expectations into the tests?

@phil-davis
Copy link
Contributor

@phil-davis Do you know what needs to be done to bring the new expectations into the tests?

I can adjust the tests to make them pass. But by doing that, I will be assuming that the config changes in this PR are correct.

Someone needs to say that the config.php diff is correct:
https://github.com/owncloud/owncloud-updater-server/pull/17/files#diff-f55db4bc294bbb90108ccfcbdd11881f4707473ad9384b382e2a6b653e118471

@jnweiger
Copy link
Author

Hehe, seems we are blocked here.
I'd suggest to unblock, we go forward and assume it is correct, unless somebody objects.
(We already have @VicDeo here -- without objections, that is probably the best expert we can get right now)

@phil-davis phil-davis self-assigned this Jan 25, 2022
@phil-davis phil-davis requested review from phil-davis and removed request for VicDeo January 26, 2022 09:46
@phil-davis
Copy link
Contributor

@jnweiger @VicDeo I made the tests pass, and added one for 10.9.*
Please review and see if this looks good now.

@phil-davis
Copy link
Contributor

@jnweiger @VicDeo ping

@VicDeo
Copy link
Member

VicDeo commented Feb 17, 2022

@phil-davis well, I have no access to the owncloud mailbox where I used to read GH notifications so it's pretty useless to ping me :)
Another funny thing that I have no push access to this repo anymore.

Anyway, your recent fixes are looking good except the daily channel update flow.
For the daily channel 10.9.0 should be updated to 10.9.1 while everything else should be updated to the daily master build.

Feel free to apply this patch to achieve this:

From baec6b43754416ac370ad179ae7d4fd397efbf42 Mon Sep 17 00:00:00 2001
From: Viktar Dubiniuk <[email protected]>
Date: Fri, 18 Feb 2022 00:10:30 +0300
Subject: [PATCH] Minor fixes for daily channel

---
 config/config.php                             | 18 +++++++-------
 .../integration/features/update.daily.feature | 24 +++++++++++++++++--
 2 files changed, 31 insertions(+), 11 deletions(-)

diff --git a/config/config.php b/config/config.php
index 8e889f0..d6c7a4a 100644
--- a/config/config.php
+++ b/config/config.php
@@ -120,19 +120,19 @@ return [
        ],
        'daily' => [
                // 10.8 should be updated to 10.9.1 and not to daily master
-               // 10.9 should be updated to 10.9.1 as well
-               // 10.9.1 should be updated to daily master instead
-               '10.8' => [
-                       'latest' => '10.9.1',
-                       'web' => 'https://doc.owncloud.com/server/10.8/admin_manual/maintenance/upgrading/update.html',
-               ],
+               // 10.9.0 should be updated to 10.9.1 as well
+               // 10.9.* should be updated to daily master instead
                '10.9' => [
+                       'downloadUrl' => 'https://download.owncloud.org/community/owncloud-daily-master.zip',
+                       'web' => 'https://doc.owncloud.com/server/latest/admin_manual/maintenance/upgrading/update.html',
+               ],
+               '10.9.0' => [
                        'latest' => '10.9.1',
                        'web' => 'https://doc.owncloud.com/server/10.9/admin_manual/maintenance/upgrading/update.html',
                ],
-               '10.9.1' => [
-                       'downloadUrl' => 'https://download.owncloud.org/community/owncloud-daily-master.zip',
-                       'web' => 'https://doc.owncloud.com/server/latest/admin_manual/maintenance/upgrading/update.html',
+               '10.8' => [
+                       'latest' => '10.9.1',
+                       'web' => 'https://doc.owncloud.com/server/10.8/admin_manual/maintenance/upgrading/update.html',
                ],
        ],
        // to prevent individual channels from bloating all upgrade path common for all channels go below
diff --git a/tests/integration/features/update.daily.feature b/tests/integration/features/update.daily.feature
index ae517ba..f01422f 100644
--- a/tests/integration/features/update.daily.feature
+++ b/tests/integration/features/update.daily.feature
@@ -1,16 +1,36 @@
 Feature: Testing the update scenario of releases on the daily channel
 ##### Please always order by version number descending #####
   ##### Tests for 10.9 should go below #####
-  Scenario: Updating an outdated-dated ownCloud 10.9 daily
+  Scenario: Updating an outdated-dated ownCloud 10.9.100 daily
     Given There is a release with channel "daily"
     And The received version is "10.9.100"
     And the received build is "2021-03-19T18:44:30+00:00"
     When The request is sent
     Then The response is non-empty
     And Update to version "100.0.0.0" is available
+    And URL to download is "https://download.owncloud.org/community/owncloud-daily-master.zip"
+    And URL to documentation is "https://doc.owncloud.com/server/latest/admin_manual/maintenance/upgrading/update.html"
+
+  Scenario: Updating an outdated-dated ownCloud 10.9 daily
+    Given There is a release with channel "daily"
+    And The received version is "10.9.1"
+    And the received build is "2021-03-19T18:44:30+00:00"
+    When The request is sent
+    Then The response is non-empty
+    And Update to version "100.0.0.0" is available
+    And URL to download is "https://download.owncloud.org/community/owncloud-daily-master.zip"
+    And URL to documentation is "https://doc.owncloud.com/server/latest/admin_manual/maintenance/upgrading/update.html"
+    
+  Scenario: Updating an outdated-dated ownCloud 10.9 daily
+    Given There is a release with channel "daily"
+    And The received version is "10.9.0"
+    And the received build is "2021-03-19T18:44:30+00:00"
+    When The request is sent
+    Then The response is non-empty
+    And Update to version "100.0.0.0" is available
     And URL to download is "https://download.owncloud.org/community/owncloud-10.9.1.zip"
     And URL to documentation is "https://doc.owncloud.com/server/10.9/admin_manual/maintenance/upgrading/update.html"
-
+  
   ##### Tests for 10.8 should go below #####
   Scenario: Updating an outdated-dated ownCloud 10.8 daily
     Given There is a release with channel "daily"
-- 
2.34.1

And it's good to go after squashing the commits.

I'm unable to review the respective PR on Gitea so I've tried to run make test-production on this branch.
It seems to be already merged and have the same flaw:
Update candidate for daily master instance is 10.9.1 which is pretty wrong.

jnweiger added a commit that referenced this pull request May 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants