OCPBUGS-15740: Fix show-config command output #2013
Conversation
openshift-ci-robot
added
jira/valid-reference
|
@pacevedom: This pull request references Jira Issue OCPBUGS-15740, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
openshift-ci
bot
added
the
approved
|
/cc @dhellmann @pmtk |
|
/test ci/prow/microshift-e2e |
|
@pacevedom: The specified target(s) for
The following commands are available to trigger optional jobs:
Use
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/test microshift-e2e |
|
/jira refresh |
openshift-ci-robot
added
jira/valid-bug
|
@pacevedom: This pull request references Jira Issue OCPBUGS-15740, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
pkg/config/files.go
Outdated
There was a problem hiding this comment.
This logic doesn't look right.
If we're going to cache the entire set of config somewhere, then we always need to update those settings in case the user has modified the input file.
Maybe we can focus this change on just reading the nodename file and using that value for the host name override?
There was a problem hiding this comment.
This is a bit open to interpretation I think. The microshift show-config command should return the active config when microshift is running, right? Which means the configuration that is currently loaded. If you modify the config file then that wont take effect until next restart, which will persist the file again. But before restarting it any changes a user may do are not visible to microshift and displaying them in show-config might be misleading?
There was a problem hiding this comment.
Yes, you're interpreting the command's purpose differently than it was originally intended.
It was meant to show the combination of what is in /etc/microshift/config.yaml and any defaults that are not explicitly in the file. You are correct that the results may not be the same as what the current process is using, of course. Those settings are logged, so it's easy to get them from journalctl. It's harder to look at a partial configuration file and understand what other settings will be added to make up the full configuration.
There was a problem hiding this comment.
Then you are right. I changed the logic to copy the run behavior and update it accordingly.
What I find misleading is the name of the command and the comment, but we can take that in a different PR.
pacevedom
force-pushed
the
OCPBUGS-15740
branch
2 times, most recently
from
a050418 to
28bebaa
Compare
Command `microshift show-config` shows the combination of what's in `/etc/microshift/config.yaml` and the defaults that are not in the file. In the event of a hostname change this file does not show the correct hostnameOverride and subjectAltNames, as they are changed right before starting microshift to preserve the original node name. This fix runs the same logic when showing the configuration or when starting microshift.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dhellmann, pacevedom The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@pacevedom: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
@pacevedom: Jira Issue OCPBUGS-15740: All pull requests linked via external trackers have merged: Jira Issue OCPBUGS-15740 has been moved to the MODIFIED state. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
4 participants
Command
microshift show-configmust show the running configuration. This command loaded the configuration from /etc/microshift/config.yaml if it existed, possibly showing wrong information (editing the file without restarting microshift yields wrong data).Having no configuration file fills the config with defaults. This might also be wrong in the event of a hostname change, when this is automatically fixed to use the previous name. This command would show the new name when MicroShift is using the previous one.
New approach stores the running configuration upon start and returns it when asked for it.
If MicroShift is not running this command returns the last active config.
Which issue(s) this PR addresses:
Closes #