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

Charm ports are configuration options, but maybe they should not #1003

Open
DnPlas opened this issue Aug 1, 2024 · 1 comment
Open

Charm ports are configuration options, but maybe they should not #1003

DnPlas opened this issue Aug 1, 2024 · 1 comment
Labels
enhancement New feature or request

Comments

@DnPlas
Copy link
Contributor

DnPlas commented Aug 1, 2024

Context

Many CKF charms have the port configuration option, which is problematic because of the following:

  1. The configuration option is not consistent, in some it refers to the http_port, in some others it refers to the metrics_port, and the description in the configuration file is not clearly defining this. For example in dex-auth:
  port:
    type: int
    default: 5556
    description: Listening port
  1. In many cases, the configuration option won't have any effect on the workload itself, only on certain things the charm could be doing with it, like when the KubernetesServicePatch is used. For example, in dex-auth, if changed, the configuration option will just affect the behaviour of the KubernetesServicePatch, but not the workload service as it is not changing anything in the command of the pebble layer.

In some other cases, like pipelines, the port is hardcoded, making it impossible to change.

  1. It may add complexity to the port configuration as there could be many options, for example a HTTP and a HTTP port.

  2. The use of this configuration option is inconsistent, some charms will have it while some others won't.

Proposal

To eliminate the option to configure ports in all CKF charms and refactor the charm code to avoid using them, unless there is a compelling reason to keep them.

Related issues:

What needs to get done

  1. Remove the port config option from all CKF charms
  2. Remove all logic/code that requires self.config['port'] or similar
  3. Update tests cases that configured this option
  4. Update any documentation that mentioned this configuration option

Definition of Done

The port is not configurable anymore for any of the CKF charms.

@DnPlas DnPlas added the enhancement New feature or request label Aug 1, 2024
Copy link

Thank you for reporting us your feedback!

The internal ticket has been created: https://warthogs.atlassian.net/browse/KF-6079.

This message was autogenerated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant