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

use official docker container for federaated servers #32413

Closed
wants to merge 11 commits into from
Closed

use official docker container for federaated servers #32413

wants to merge 11 commits into from

Conversation

patrickjahns
Copy link
Contributor

Description

Use official docker container in federated tests

Related Issue

Motivation and Context

Fix CI

How Has This Been Tested?

🤖

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

Open tasks:

@patrickjahns patrickjahns self-assigned this Aug 22, 2018
individual-it
individual-it requested changes Aug 22, 2018
View reviewed changes
.drone.yml Outdated
@@ -337,6 +337,7 @@ pipeline:
pull: true
environment:
- TEST_SERVER_URL=http://server
- TEST_SERVER_FED_URL=http://federated"
Copy link
Member

Choose a reason for hiding this comment

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

that will make run.sh to try to set various settings on the fed. server, in test cases where it was not started it fails.
that was the reason to use echo "export TEST_SERVER_FED_URL=http://federated" > /drone/saved-settings.sh

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can work with using an environment parameter that is empty - that would avoid the necessity to share on file system level

.drone.yml Outdated
when:
matrix:
USE_FEDERATED_SERVER: true
# install-fed-server:
Copy link
Member

Choose a reason for hiding this comment

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

why not deleting those lines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Was quickly prototyping - will be deleted when review-ready

This was referenced Aug 22, 2018
Merged
Merged
@patrickjahns
Copy link
Contributor Author

Issue I am facing now is, that the testing repository would either needed to be moved in a subdirectory to pull the latest changes - or we misuse a github release tag ( 'latest' ) - and re-release the app always on that tag

@individual-it
Copy link
Member

I would prefer to misuse the tag

@codecov
Copy link

codecov bot commented Aug 23, 2018 •
edited
Loading

Codecov Report

Merging #32413 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #32413   +/-   ##
=========================================
  Coverage     64.83%   64.83%           
  Complexity    18319    18319           
=========================================
  Files          1198     1198           
  Lines         69559    69559           
  Branches       1282     1282           
=========================================
  Hits          45100    45100           
  Misses        24086    24086           
  Partials        373      373
Flag Coverage Δ Complexity Δ
#javascript 53.08% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 66.19% <ø> (ø) 18319 <ø> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d7a09c4...00c5ac5. Read the comment docs.

@patrickjahns patrickjahns force-pushed the master_user_federated_docker branch 5 times, most recently from 84a6ea2 to f0aa89b Compare August 23, 2018 13:55
@patrickjahns
Copy link
Contributor Author

@ownclouders rebase

@ownclouders
Copy link
Contributor

Hey! I'm GitMate.io! This pull request is being rebased automatically. Please DO NOT push while rebase is in progress or your changes would be lost permanently ⚠️

@ownclouders
Copy link
Contributor

Automated rebase failed! Please rebase your pull request manually via the command line.

Reason:

Command: git rebase base/master

Exit Code: 128

Cause:

error: Failed to merge in the changes.

@patrickjahns patrickjahns force-pushed the master_user_federated_docker branch 3 times, most recently from be02985 to f25b730 Compare August 29, 2018 11:37
@patrickjahns
Copy link
Contributor Author

Parts of this PR are superseded by #32505 - but once the Guzzle part is finished, we can move with docker containers as federated servers

@patrickjahns
Copy link
Contributor Author

Rebased to fetch latest changes from core/master - afaik we do not require the guzzle part with ssl certificate anylonger

@patrickjahns patrickjahns mentioned this pull request Aug 31, 2018
Open
@patrickjahns patrickjahns force-pushed the master_user_federated_docker branch 2 times, most recently from 3b77344 to 8f1e8b9 Compare August 31, 2018 19:54
@PVince81 PVince81 added this to the backlog milestone Feb 8, 2019
@PVince81
Copy link
Contributor

PVince81 commented Feb 8, 2019

@phil-davis please evaluate if we should continue this and reschedule

@patrickjahns
Copy link
Contributor Author

Went over the current ideas and have the following remarks:

  • DIND is not necessary to start the owncloud container when we don't need to add/edit/mount files directly in the container
federated:
    image: owncloud/server:latest
    pull: true
    detach: true
    environment:
      - OWNCLOUD_APPS_INSTALL=https://github.com/owncloud/testing/releases/download/latest/testing.tar.gz
      - OWNCLOUD_APPS_ENABLE=testing
    when:
      matrix:
        TEST_FEDERATION: true

Now here is the caveat - what type of onwcloud remote instance do we want to test with federation? Following topics need some more thought:

  • should the federated server have redis ( file locking ) or not?
  • should we have more than one database system on the federated system ?

Regarding the issue with ssl:
When you continue to use dind - you could mount a custom apache config and also ssl certificates inside the docker containers and start it with ssl

@individual-it
Copy link
Member

blocked by #34483

@individual-it
Copy link
Member

seems pretty complicated to me, closing this in favour of the discussion here #34500
We can reopen this PR or reuse the code if needed

@individual-it individual-it deleted the master_user_federated_docker branch February 15, 2019 08:59
@lock lock bot locked as resolved and limited conversation to collaborators Feb 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@individual-it individual-it individual-it requested changes

@phil-davis phil-davis Awaiting requested review from phil-davis

Assignees

@individual-it individual-it

Labels
2 - Developing
Projects
None yet
Milestone
backlog
Development

Successfully merging this pull request may close these issues.
5 participants
@patrickjahns @individual-it @ownclouders @PVince81 @phil-davis