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

Maple Upgrade #11

Closed
wants to merge 5 commits into from
Closed

Conversation

BbrSofiane
Copy link
Member

@BbrSofiane BbrSofiane commented Nov 21, 2021

  • In the plugin’s setup.py, bump the “tutor” version that the plugin depends on.
  • In the __about__.py module, bump the version number to the next major release
  • Replace all instances of the previous release name (“lilac”) with the new one (“maple”) in the entire repo: git grep -i lilac.
  • Nginx-specific patches should be deleted, as we no longer run Nginx in Maple (see PR 2)
  • Containers should run in unprivileged mode on Kubernetes (see PR 1)
  • Upgrade server and client images
  • MINIO_ACCESS_KEY and MINIO_SECRET_KEY are deprecated. Please use MINIO_ROOT_USER and MINIO_ROOT_PASSWORD

Tests

  • Dev machine using tutor local
  • Dev machine using tutor dev
  • Dev machine using tutor k8s
  • Remote server using tutor local and HTTPS
  • Remote server using tutor local, HTTPS and S3 gateway

@BbrSofiane
Copy link
Member Author

BbrSofiane commented Nov 21, 2021

Embedded MinIO Browser removed replaced with MinIO Console project - tutor users should be aware that the UI will be different.

@@ -1,6 +1,6 @@
minio:
ports:
- "127.0.0.1:9000:9000"
- "127.0.0.1:9001:9001"
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 could you amend the README accordingly?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

Choose a reason for hiding this comment

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

See my comment above: we might have to expose both the 9000 and the 9001 ports.

README.rst Outdated Show resolved Hide resolved
{{ MINIO_HOST }}{% if not ENABLE_HTTPS %}:80{% endif %} {
reverse_proxy nginx:80
{{ MINIO_HOST }}{$default_site_port} {
import proxy "minio:9001"
Copy link
Contributor

Choose a reason for hiding this comment

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

Wait... What is the difference between the server and the console ports? If I understand correctly, the console port is for accessing the web UI, right? The problem is that other services, such as the LMS/CMS, need to be able to access the server from the https://minio.mylms.com address. So the MINIO_HOST should point to the minio server, and not the console.

What this means is that we might need to expose the MinIO console at a different web host. Something like:

{{ MINIO_CONSOLE_HOST }}{$default_site_port} {
    import proxy "minio:9000"
{{ MINIO_CONSOLE_HOST }}{$default_site_port} {
    import proxy "minio:9001"
}

Do you understand my point?

Copy link
Contributor

@regisb regisb left a comment

Choose a reason for hiding this comment

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

I tested this PR and failed to upload media files. I can attempt to make changes to make it work. It probably means running the MinIO console at a different domain name.

@@ -1,7 +1,7 @@
# MinIO
minio-job:
image: {{ MINIO_MC_DOCKER_IMAGE }}
image: { { MINIO_MC_DOCKER_IMAGE } }
Copy link
Contributor

Choose a reason for hiding this comment

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

The space between { { is causing errors on my machine.

@regisb
Copy link
Contributor

regisb commented Dec 16, 2021

Closed by #12

@regisb regisb closed this Dec 16, 2021
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.

None yet

2 participants