Rishabh Dave [Tue, 8 Dec 2020 05:17:52 +0000 (10:47 +0530)]
qa/vstart_runner: inherit methods instead of duplicating them
Inherit methods run_ceph_w(), run_cluster_cmd(), raw_cluster_cmd() and
raw_cluster_cmd_result() from ceph_manager.CephManager in
vstart_runner.LocalCephManager instead of duplicating them.
Rishabh Dave [Tue, 8 Dec 2020 05:10:35 +0000 (10:40 +0530)]
qa/ceph_manager: make it possible to reuse few methods
Make minor adjustments to ceph_manager.CephManager so that methods
run_ceph_w(), run_cluster_cmd() raw_cluster_cmd() and
raw_cluster_cmd_result() can be reused, instead of duplicating, in
subclasses. The adjustments are -
* Having variables contain arguments that'll be prepended to every
command received by the methods above.
* Grouping variables that needs to be overridden together so that it is
easy to spot and override them for users.
Rishabh Dave [Fri, 30 Jul 2021 16:00:02 +0000 (21:30 +0530)]
qa/vstart_runner: don't use "shell=False" in run_ceph_w()
Instead prepend "exec sudo" to the command arguments of
LocalCephManager.run_ceph_w(). This makes the default parameter
"shell=False" redundant in case of
ceph_manager.CephManager.run_ceph_w(), so get rid of it too and update
calls to run_ceph_w() accordingly.
The reason behind using any of these workarounds is that running "ceph
-w" with "shell" set to True leads to crash for Ceph API CI job. See
this ticket for more details: https://tracker.ceph.com/issues/49644.
The reason behind switching the workaround is that in the following
commits to reduce duplication LocalCephManager.run_ceph_w() will be
deleted and CephManager.run_ceph_w() will be used by LocalCephManager
via inheritance. However, due to the issue described above, Ceph API
test will fail since "shell" is set to "True" for the command issued by
CephManager.run_ceph_w(). Prepending "exec sudo" to the command when it
is used in LocalCephManager makes this duplication unnecessary and also
prevents Ceph API test from failing.
replace strict_iecstrtoll(const char *str,..) with
strict_iecstrtoll(std::string_view, ..). which is more convenient.
and both of them share the same implementation:
strict_sistrtoll() is but an alias of strict_si_cast<uint64_t>(..),
let's just drop the former. there are way too many thin wrappers in
strtol.{h,cc}. they don't offer lots of benefit to us.
this variant is better than strict_si_cast(const char*), because:
* we can just pass std::string to it, as std::string_view can be
constructed from a std::string implicitly
* strict_si_cast(std::string_view, ..) is the underlying
implementation of strict_si_cast(const char*,..), so less
indirection helps with the readability.
rgw,mon,common/strtol: use strict_iec_cast(std::string_view, ..)
this variant is better than strict_iec_cast(const char*), because:
* we can just pass std::string to it, as std::string_view can be
constructed from a std::string implicitly
* strict_iec_cast(std::string_view, ..) is the underlying
implementation of strict_iec_cast(const char*,..), so less
indirection helps with the readability.
* rgw,mon: use strict_iec_cast(std::string_view, ..) instead.
vstart.sh: set objectstore to "cyanstore" if --cyanstore is specified
in e6ed65db8b4e0a2f8026c2e35a12dd292c5f2b8c, "cyanstore" is added to the
help message of vstart.sh, but we should also check for this option, and
set the ceph option accordingly.
in this change, the option is checked and "objectstore" is updated
accordingly.
In file included from ../src/common/config.h:27,
from ../src/crimson/common/config_proxy.h:8,
from ../src/crimson/osd/main.cc:23:
../src/common/config_values.h: In copy constructor ‘ConfigValues::ConfigValues(const ConfigValues&)’:
../src/common/config_values.h:19:7: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
19 | class ConfigValues {
| ^~~~~~~~~~~~
because fmt is packaged in EPEL, while librados is packaged
in RHEL, so we cannot have fmt as a runtime dependency of librados.
to address this issue, we should compile librados either with static library
or with header-only library of fmt. but because the fedora packaging
guideline does no encourage us to package static libraries, and it would
be complicated to package both static and dynamic library for fmt.
the simpler solution would be to compile Ceph with the header-only
version of fmt.
in this change, we compile ceph with the header-only version of fmt
on RHEL to address the runtime dependency issue.
* an interface library named "fmt-header-only" is introduced. it brings
the support to the header only fmt library.
* fmt::fmt is renamed to fmt
* an option named "WITH_FMT_HEADER_ONLY" is introduced
* fmt::fmt is an alias of "fmt-header-only" if "WITH_FMT_HEADER_ONLY"
is "ON", and an alias of "fmt" otherwise.
because fmt is packaged in EPEL, while librados is packaged
in RHEL, so we cannot have fmt as a runtime dependency of librados.
to address this issue an option "WITH_FMT_HEADER_ONLY" is introduced, so
that we can enable it when building Ceph with the header version of fmt.
and the built packages won't have runtime dependency of fmt.
We don't need to run an extra command (mgr module ls) to obtain the mgr
modules list since we already have this information in the mgr_map.
This workflow is already done for the monitoring stack or for configuring
the iscsi integration within the dashboard (during creation) via the
config_dashboard method.
The mgr_map is mocked in the tests with the dashboard module enabled so we
don't need _mon_command_mock_mgr_module_ls anymore.
rpm: drop use of $FIRST_ARG in ceph-immutable-object-cache
The use of $FIRST_ARG was probably required because the SUSE-specific
%service_* rpm macros were playing tricks on the shell positional parameters.
This is bad practice and error-prone, so let's assume that no macros should do
that anymore and hence it's safe to assume that positional parameters remain
unchanged after any rpm macro call.
we use Findthrift.cmake method for adding thrift dependencies, the
cleanup 80e82686ebafe36fca6dfd21cb32e63ced94d5cd missed removing these
methods intended to buildthrift from source.
this address the missing failure due to ,
```
CMake Error at cmake/modules/BuildJaeger.cmake:61 (include):
include could not find load file:
Since inject_facts_as_vars is set to false in the ansible.cfg file then we
have to update the references to use ansible_facts[<thing>] instead of
ansible_<thing>.
We already install the dependency from ceph-ansible requirements.txt and to
avoid false positive (like after rebooting a node) we can retry failing test.
Without loading the ansible.cfg file from ceph-ansible project, we don't
have the pipelining enabled which can result in significant performance
improvement.
This removes the ANSIBLE_ACTION_PLUGINS, ANSIBLE_RETRY_FILES_ENABLED and
ANSIBLE_SSH_RETRIES environment variables as it is already included in the
ansible.cfg file.
ceph-volume/tests: update ansible ssh_args env var
The ansible ssh_args parameter is usually defined in the ansible.cfg file.
Currently this variable is overrided in tox to manage the vagrant ssh file
but we lost all default values.
pybind/mgr/dashboard: build frontend/dist in build
to enable us to build out of source, we should build the frontend
artifacts out of src. in this change:
* cmake:
- install frontend/dist from build
- drop rules to exclude unused frontend artifacts.
- pass `--output-path` option to ng
- copy `frontend/package.json` to build so that the frontend
can find it
* pybind/mgr/dashboard/module.py: fall back to build directory
if frontend/dist is not found.
* pybind/mgr/dashboard/__init__.py: use frontend/dist when performing
unit test.
instead of installing the whole directory, install mgr modules
individually, so it's more clear that what EXCLUDE pattern is
used in which mgr module.
this improves the maintability and readability. in future, we
should only install the artifacts. with this change, we can
specify the include pattern only for dashboard mgr module so
that, for instance, only .js, .html and .css files are installed.
updates:
* custom target jaeger_base
* target jaeger-base which encapsulates all jaeger libs
* include external libraries path needed linking libraries and including
opentracing and jaegertracing headers files
cmake: change debian DESTDIR for installing jaeger deps
debian uses debian/tmp as destination dir for installing build files,
but since we are using common path($build_dir/external) available both
for rpm and debian based dependency installation, it becomes far more
complicated to maintain include/link path for these external projects.
elaborating on it:
path we are configuring for both rpm and debian installing including,
and linking of external librarires:
/build/ceph-17.0.0-5779-g928f9e55/obj-x86_64-linux-gnu/external/
debian appends DESTDIR to this path, and hence our predefined target
artificats cannot find correct path for external libs, I tried adding
ENV${DESTDIR} so that it could include correct external lib install
path, but it still cannot find them:
failed to link in case of:
- install(DIRECTORY $ENV{DESTDIR}${CMAKE_BINARY_DIR}/external/include/jaegertracing
- $ENV{DESTDIR}${CMAKE_BINARY_DIR}/external/include/opentracing
- DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
- include_directories(SYSTEM ${CMAKE_INSTALL_INCLUDEDIR}/jaegertracing)
- include_directories(SYSTEM ${CMAKE_INSTALL_INCLUDEDIR}/opentracing)