Skip to content

cephadm: Make ceph-iscsi api user and password mandatory#35097

Merged
tchaikov merged 1 commit intoceph:masterfrom
matthewoliver:cephadm_iscsi_api_user_password
May 27, 2020
Merged

cephadm: Make ceph-iscsi api user and password mandatory#35097
tchaikov merged 1 commit intoceph:masterfrom
matthewoliver:cephadm_iscsi_api_user_password

Conversation

@matthewoliver
Copy link
Contributor

The api user and password is required in order to use the API so let's
make these mandatory. The ceph orch daemon add iscsi now has them
mandatory:

ceph orch daemon add iscsi <api_user> <api_password>

If your using apply with a yaml file, the validate in the spec now
checks for these too.

Signed-off-by: Matthew Oliver moliver@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 dashboard backend
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

@matthewoliver matthewoliver requested a review from a team as a code owner May 18, 2020 08:15
@matthewoliver matthewoliver force-pushed the cephadm_iscsi_api_user_password branch from 9bfae5f to b75a5fd Compare May 18, 2020 08:18
@matthewoliver
Copy link
Contributor Author

Just corrected commit message

@sebastian-philipp
Copy link
Contributor

=================================== FAILURES ===================================
_______________ TestCephadm.test_apply_save[spec10-apply_iscsi] ________________

self = <cephadm.tests.test_cephadm.TestCephadm object at 0x7f99d04987f0>
spec = IscsiServiceSpec({'placement': PlacementSpec(count=1, hosts=[HostPlacementSpec(hostname='test', network='', name='')])...None, 'api_port': None, 'api_user': None, 'api_password': None, 'api_secure': None, 'ssl_cert': None, 'ssl_key': None})
meth = <function CephadmOrchestrator.apply_iscsi at 0x7f99e2736bf8>
cephadm_module = <cephadm.module.CephadmOrchestrator object at 0x7f99d04ae2e8>

    @pytest.mark.parametrize(
        "spec, meth",
        [
            (ServiceSpec('mgr'), CephadmOrchestrator.apply_mgr),
            (ServiceSpec('crash'), CephadmOrchestrator.apply_crash),
            (ServiceSpec('prometheus'), CephadmOrchestrator.apply_prometheus),
            (ServiceSpec('grafana'), CephadmOrchestrator.apply_grafana),
            (ServiceSpec('node-exporter'), CephadmOrchestrator.apply_node_exporter),
            (ServiceSpec('alertmanager'), CephadmOrchestrator.apply_alertmanager),
            (ServiceSpec('rbd-mirror'), CephadmOrchestrator.apply_rbd_mirror),
            (ServiceSpec('mds', service_id='fsname'), CephadmOrchestrator.apply_mds),
            (RGWSpec(rgw_realm='realm', rgw_zone='zone'), CephadmOrchestrator.apply_rgw),
            (NFSServiceSpec('name', pool='pool', namespace='namespace'), CephadmOrchestrator.apply_nfs),
            (IscsiServiceSpec('name', pool='pool'), CephadmOrchestrator.apply_iscsi),
        ]
    )
    @mock.patch("cephadm.module.CephadmOrchestrator._run_cephadm", _run_cephadm('{}'))
    def test_apply_save(self, spec: ServiceSpec, meth, cephadm_module):
        with self._with_host(cephadm_module, 'test'):
            spec.placement = PlacementSpec(hosts=['test'], count=1)
            c = meth(cephadm_module, spec)
>           assert wait(cephadm_module, c) == f'Scheduled {spec.service_name()} update...'

cephadm/tests/test_cephadm.py:484: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
cephadm/tests/fixtures.py:69: in wait
    raise_if_exception(c)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

c = <class 'cephadm.module.CephadmCompletion'>(_s=3, val='_exception: Cannot add ISCSI: No Api user specified', _on_c=<fun...completion.<locals>.wrapper.<locals>.<lambda> at 0x7f99d04a7bf8>, id=140298904043472, name=<lambda>, pr=NA, _next=None)

    def raise_if_exception(c):
        # type: (Completion) -> None
        """
        :raises OrchestratorError: Some user error or a config error.
        :raises Exception: Some internal error
        """
        if c.serialized_exception is not None:
            try:
                e = pickle.loads(c.serialized_exception)
            except (KeyError, AttributeError):
                raise Exception('{}: {}'.format(type(c.exception), c.exception))
>           raise e
E           ceph.deployment.service_spec.ServiceSpecValidationError: Cannot add ISCSI: No Api user specified

orchestrator/_interface.py:633: ServiceSpecValidationError
------------------------------ Captured log call -------------------------------
ERROR    orchestrator._interface:_interface.py:321 _Promise failed
Traceback (most recent call last):
  File "/home/jenkins-build/build/workspace/ceph-pull-requests/src/pybind/mgr/orchestrator/_interface.py", line 274, in _finalize
    next_result = self._on_complete(self._value)
  File "/home/jenkins-build/build/workspace/ceph-pull-requests/src/pybind/mgr/cephadm/module.py", line 138, in <lambda>
    return CephadmCompletion(on_complete=lambda _: f(*args, **kwargs))
  File "/home/jenkins-build/build/workspace/ceph-pull-requests/src/pybind/mgr/cephadm/module.py", line 1902, in apply_iscsi
    return self._apply(spec)
  File "/home/jenkins-build/build/workspace/ceph-pull-requests/src/pybind/mgr/cephadm/module.py", line 1862, in _apply
    get_daemons_func=self.cache.get_daemons_by_service,
  File "/home/jenkins-build/build/workspace/ceph-pull-requests/src/pybind/mgr/cephadm/schedule.py", line 77, in validate
    self.spec.validate()
  File "/home/jenkins-build/build/workspace/ceph-pull-requests/src/python-common/ceph/deployment/service_spec.py", line 611, in validate
    'Cannot add ISCSI: No Api user specified')
ceph.deployment.service_spec.ServiceSpecValidationError: Cannot add ISCSI: No Api user specified
=============================== warnings summary ===============================

The api user and password is required in order to use the API so let's
make these mandatory. The `ceph orch daemon add iscsi` now has them
mandatory:

  ceph orch daemon add iscsi <pool> <api_user> <api_password>

If your using apply with a yaml file, the validate_add in the spec now
checks for these too.

Signed-off-by: Matthew Oliver <moliver@suse.com>
@matthewoliver matthewoliver force-pushed the cephadm_iscsi_api_user_password branch from b75a5fd to a36165b Compare May 19, 2020 00:18
@matthewoliver
Copy link
Contributor Author

Opps, sorry, pushed the code up last night and forgot about the tests that weren't added and amended to this patch so didn't go up with it.

@mgfritch
Copy link
Contributor

Comment on lines +609 to +614
if not self.api_user:
raise ServiceSpecValidationError(
'Cannot add ISCSI: No Api user specified')
if not self.api_password:
raise ServiceSpecValidationError(
'Cannot add ISCSI: No Api password specified')
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 going to get interesting if we're making an upgrade and users already have an iscsi service applied without user / password.

Is there a super easy way to resolve this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants