Skip to content

cephadm: cephadm ls broken for SUSE downstream alertmanager container#39722

Merged
sebastian-philipp merged 3 commits intoceph:masterfrom
p-se:wip-pse-cephadm-SUSE-alertmanager
Mar 2, 2021
Merged

cephadm: cephadm ls broken for SUSE downstream alertmanager container#39722
sebastian-philipp merged 3 commits intoceph:masterfrom
p-se:wip-pse-cephadm-SUSE-alertmanager

Conversation

@p-se
Copy link
Contributor

@p-se p-se commented Feb 26, 2021 •
edited
Loading

Fixes an error with cephadm ls when SUSE's downstream alertmanager is used. The difference here is that the executable is named prometheus-alertmanager instead of alertmanager as in the Prometheus upstream image.

#==[ Command ]======================================#
# /usr/sbin/cephadm ls
Non-zero exit code 127 from /usr/bin/podman exec 81581144900c6b7f5a1da7124bd0debb26cd455349f162729b4ea3e9f7fc4f86 alertmanager --version
/usr/bin/podman:stderr Error: exec failed: container_linux.go:349: starting container process caused "exec: \"alertmanager\": executable file not found in $PATH": OCI runtime command not found error
[
...
    {
        "style": "cephadm:v1",
        "name": "alertmanager.ses-5-admin",
        "fsid": "33ec5433-261d-3dad-b75a-cd21d35a0946",
        "systemd_unit": "ceph-33ec5433-261d-3dad-b75a-cd21d35a0946@alertmanager.ses-5-admin",
        "enabled": true,
        "state": "running",
        "container_id": "81581144900c6b7f5a1da7124bd0debb26cd455349f162729b4ea3e9f7fc4f86",
        "container_image_name": "foo.bar.de:5000/registry.suse.com/caasp/v4.5/prometheus-alertmanager:0.16.2",
        "container_image_id": "4683615b36cb2f4fb8580ae7ebbe0cdf42db25ae79c7ca3f48f5096706c25679",
        "version": null,
        "started": "2021-02-23T17:25:13.927267",
        "created": "2021-01-20T17:02:49.987979",
        "deployed": "2021-01-22T15:49:37.746184",
        "configured": "2021-01-22T15:49:38.822172" 
    },
...
]

Fixes: https://tracker.ceph.com/issues/49506

Signed-off-by: Patrick Seidensal pseidensal@suse.com

Checklist

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug
Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

@p-se p-se requested a review from a team as a code owner February 26, 2021 16:40
mgfritch
mgfritch reviewed Feb 26, 2021
View reviewed changes
Copy link
Contributor

@mgfritch mgfritch left a comment

Choose a reason for hiding this comment

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

this function is already becoming incredibly hard to read ...

any chance we can move the version logic up into a staticmethod of the Monitoring class?

ceph/src/cephadm/cephadm

Lines 362 to 373 in 80fb753

@staticmethod
def get_version(ctx, container_id):
# type: (CephadmContext, str) -> Optional[str]
version = None
out, err, code = call(ctx,
[ctx.container_path, 'exec', container_id,
NFSGanesha.entrypoint, '-v'])
if code == 0:
match = re.search(r'NFS-Ganesha Release\s*=\s*[V]*([\d.]+)', out)
if match:
version = match.group(1)
return version

p-se added 2 commits March 1, 2021 15:35
@p-se
cephadm: cephadm ls broken for SUSE downstream alertmanager container
3c222a8 
Fixes: https://tracker.ceph.com/issues/49506

Signed-off-by: Patrick Seidensal <pseidensal@suse.com>
@p-se
cephadm: remove unused imports
0d55626 
Signed-off-by: Patrick Seidensal <pseidensal@suse.com>
@p-se
Copy link
Contributor Author

p-se commented Mar 1, 2021

jenkins test make check

@sebastian-philipp sebastian-philipp added the wip-swagner-testing My Teuthology tests label Mar 1, 2021
mgfritch
mgfritch approved these changes Mar 1, 2021
View reviewed changes
Copy link
Contributor

@mgfritch mgfritch left a comment

Choose a reason for hiding this comment

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

lgtm!

@sebastian-philipp
Copy link
Contributor

sebastian-philipp commented Mar 2, 2021

https://pulpito.ceph.com/swagner-2021-03-02_10:27:34-rados:cephadm-wip-swagner-testing-2021-03-01-1657-distro-basic-smithi/

@sebastian-philipp sebastian-philipp merged commit 06258ea into ceph:master Mar 2, 2021
@sebastian-philipp sebastian-philipp mentioned this pull request Mar 3, 2021
Merged
@smithfarm smithfarm mentioned this pull request Mar 9, 2021
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mgfritch mgfritch mgfritch approved these changes

+1 more reviewer

@sebastian-philipp sebastian-philipp sebastian-philipp approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bug-fix cephadm monitoring wip-swagner-testing My Teuthology tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@p-se @sebastian-philipp @mgfritch