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

[DRAFT] Next major version #342

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

alfrunes
Copy link
Contributor

@alfrunes alfrunes commented Sep 16, 2024

Services migrated:

  • auditlogs
  • create-artifacts-worker
  • deployments
  • deviceauth
  • deviceconfig
  • deviceconnect
  • devicemonitor
  • generate-delta-worker
  • gui
  • inventory
  • iot-manager
  • tenantadm
  • useradm
  • workflows
  • api-gateway

@alfrunes
Copy link
Contributor Author

@oldgiova I created this draft with the idea of trying to simplify things as we migrate to the new repository layout (still TBD). So far, I have only focused on migrating auditlogs service.

Copy link
Contributor

@oldgiova oldgiova left a comment

Choose a reason for hiding this comment

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

It looks like a great improvement! Thank you!

Just few comment and clarification requests...

Also, please consider including #170 into this breaking change.

Also, there's to remove the unused file _image-pull-secret.yaml

- `mender-server-enterprise` if `mender.enterprise`
- `mendersoftware` otherwise.
* Default tag updated to follow AppVersion in Chart.yaml
* `username`/`password` is removed to discourage bad security practices
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

So you have to create your own secret by hand? Then we'll have to add the snippet to the docs.mender.io:

  • TODO: add a snippet like kubectl create secret docker-registry NAME --docker-username=user --docker-password=password --docker-email=email --docker-server=registry.mender.io in the Mender Docs

Comment on lines 16 to 25
* Rename options:
* `global.image` -> `default.image`
* `global.image.imagePullPolicy` -> `default.image.pullPolicy`
* `global.mongodb` -> `mender.mongodb`
* `global.nats` -> `mender.nats`
* `global.redis` -> `mender.redis`
* `global.storage` -> `mender.storage.type`
* `global.s3` -> `mender.storage.s3`
* `global.azure` -> `mender.storage.azure`
* `global.url` -> `mender.url`
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't it better to use mender.image instead of default.image?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What about simply image? I'm actually thinking of just removing the mender scope and move the global into the top-level or default scope.
The idea of putting it in the default was to apply the same logic as we do for all default values: unless there is an override, fall back on default.

Comment on lines 26 to 36
* Removed options:
- `global.image.username`
- `global.image.password`
- `global.auditlogs`: Replaced by `auditlogs.enabled`
- `global.hosted`
- `global.s3.AWS_TAG_ARTIFACT`
- `global.redis.username`: Replaced by URL (connection string)
- `global.redis.password`: Replaced by URL (connection string)
- `global.s3.AWS_SERVICE_ACCOUNT_NAME`: superseded by `mender.serviceAccount.name`
- `test.enabled`
Copy link
Contributor

Choose a reason for hiding this comment

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

This is also needed to make new NATS subchart working. However, this is quite a problematic change for customers, especially the ones that could miss the CHANGELOG and the upgrade doc.
Maybe is it useful to set a deprecation for global.image, still leaving a grace period, while both global.image and the new default.image are coexisting for some time?

Either the way, I think we should involve Eystein and the CE team for this change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then I believe we need to consider this for the other attributes as well.
So just to be clear, the order of precedence would be service.image, global.image and default.image?

@@ -1,5 +1,5 @@
apiVersion: v2
appVersion: "3.7.7"
appVersion: "v4.0.0"
description: Mender is a robust and secure way to update all your software and deploy your IoT devices at scale with support for customization
name: mender
version: 5.10.1
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
version: 5.10.1
version: 6.0.0

;)

Besides, this could be a good exercise for the release-please tool; wanna make a try?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Besides, this could be a good exercise for the release-please tool; wanna make a try?

That would be pretty neat! 🚀

# Redis as a subchart
# Using a bitnami sub-chart by default = test usage only
# It's recommended to use a suitable Redis Cluster for Production
redis:
enabled: true
enabled: false
Copy link
Contributor

Choose a reason for hiding this comment

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

So the idea is to disable it by default, for open source users? Then we have to document it for the enterprise users...

  • TODO: add redis.enabled=true to Mender Enterprise installation in the Mender Docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My impression is that all optional dependencies are opt-in. For production installations we should try to instruct users to maintain their own installations of Mongo, Redis and NATS and not rely on the chart dependencies.
But true, all these changes needs follow-up in the docs.

BREAKING CHANGE: See CHANGELOG.md
Signed-off-by: Alf-Rune Siqveland <[email protected]>
@alfrunes
Copy link
Contributor Author

I believe I overreached with this one and created #343. To only address the absolutely necessary breaking changes to the image repository and the workflows worker repository deprecation.

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.

2 participants