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

squid: cephadm: add a new SMB service to the mgr module #56898

Merged
merged 34 commits into from Apr 16, 2024

Conversation

adk3798
Copy link
Contributor

@adk3798 adk3798 commented Apr 15, 2024

Backport of #55068 and its dependency #55719

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 cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox
  • jenkins test windows
  • jenkins test rook e2e

None of the callers of map_vips ever checks for a None return. So
instead of handling any error conditions it would always just blow
up with a semi-obscure TypeError. Convert the function to always
raise an exception (one that tries to breifly explain the condition)
when something goes wrong. I also take the opportunity to make
more clearer logging and reduce an indentation level.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit 52a924e)
Nothing outside of vip.py called map_vips, so let us make sure this
is considered a private function and prefix it with the underscore.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit addc970)
While testing my previous patches were correct I noticed that the string
here was logged exactly as written, and was thus pretty useless.  This
was probably meant to be an f-string. So make it one. Also get rid of
the unnecessary map call, the list and IP address type can repr
themselves just fine IMO.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit 19e664f)
Add functions to cephadm.py that will be later used to template
strings within the yaml files in the cephadm suites. This will be used
to replace the specific subst_vip call with generic calls that let
tests access "any" variables stored on the test ctx.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit 7bd85b5)
Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit 4f1f095)
In the future, some sidecar containers will need to share namespaces
with the primary container (or each other). Make it easy to set this up
by creating a enable_shared_namespaces function and Namespace enum.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit b6fa001)
Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit d373edf)
Add an incomplete but largely viable SMB/Samba container daemon form
implementation to cephadm. Currently unused but it lays out some of the
basics needed to create smb sharing using samba containers under cephadm
orchestration.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit 0169fd9)
Enable the use of the SMB container daemon form class by importing, and
thus registering, it. Note that the only way to invoke this feature is
by hand rolling some JSON to feed to the `ceph _orch deploy` command.
Connecting this with the cephadm mgr module is left as a future task.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit f86e710)
Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit 3b0f331)
The not-a-real-fqdn hostname that the containers got were causing
performance issues joining AD (and running testjoin and winbind).
Define a virtual hostname that can be passed in from the service or
automatically derived from the system's hostname.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit f8160ed)
Tests that touch this enum fail for me locally but pass in the CI. This
seems to be due to new enum related behavior in Python 3.11.
See: https://blog.pecar.me/python-enum
Instead of fixing it as suggested in the above blog, adding a __str__
method works on all python versions I care to know about.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit 07b4490)
In the seemingly never-ending fight against line continuations and just
blatting tons of stuff onto single lines another small victory is won.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit 96456aa)
While ceph doesn't enforce sorted imports I prefer them when possible. I
had once sorted these imports but then nvmeof came along an ruined
things. Put nvmeof back in it's place.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit 35028e1)
Reformat the _service_classes variable so that it uses a multi-line list
with a single item on each line in a more black-ish style that is more
readable (especially if you use code-folding wisely).
Sort the list while we're at it.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit a500f42)
…caps

Refactor get_keyring_with_caps such that the keyring simplification code
is moved into a new function that can be used in other locations.
get_keyring_with_caps will now call the new function to return the
simplified & consistent keyring output.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit 41e2b27)
Reformat the ServiceSpec classes properties KNOWN_SERVICE_TYPES and
REQUIRES_SERVICE_ID. These were previously strings that were converted
to lists via a call to split. With a string there's very little a human
or a tool can do to validate the content. Changing these into proper
lists in the source code brings clarity of intent and the ability to
analyze the code. Because there's no semantic difference what services
are listed where (this means the type could probably be a set - a quest
for another day) I also took the opportunity to sort the contents of the
lists and add some basic comments for what these lists are for.

It also removes the use of (ugly, IMO) line continuations. The downside
is that it makes more total lines, but if that bugs you - use code
folding :-).

Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit 4fc2697)
Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit 4f655c5)
Will be used in a later commit to implement deploying smb instances.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit a88cf50)
Add the smb service by name or by type to one of the many, many touch
points in the orchestrator and cephadm packages needed to get the
orchestrator aware of smb.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit c5e4912)
Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit 3985325)
Because the "if-ladder" was only ever assigning a single variable with
a value it can be directly replaced by a dict & dict-lookup which is
much more succinct.
Also take the opportunity to sort the (non-comment) lines as there's
no meaning to the previous order and this makes it easier for a reader
to scan through.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit 0847ee2)
Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit 9a58843)
Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit 4e897de)
Add a new task function to cephadm.py that sets up a container running
the Samba based domain controller on a node using podman or docker.
Much of the function actually deals with disabling systemd-resolved
because that service conflicts with the DNS server component of the DC.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit a99dc99)
There are cases where I want to pass some large-ish strings to ceph
commands executed via cephadm shell. Allow items within the commands
list to be dicts containing a command (as before) and an optional
stdin variable. This change also supports possible future extensions as
well.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit 2a917e2)
Add a cephadm.exclude role that excludes a test node from cluster setup
and related commands. I need this as I have  test node that will be set
up as an AD Domain Controller for testing Samba and do not want that
node to be have *any* other services running on it.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit 3ec0bfa)
Add a new cephadm.exec task that works similarly to the existing
vip.exec but instead of only considering VIP related string replacements
it uses that templating feature that was recently added to the
cephadm module for generalized string templating.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit 1ed6654)
Add a `role_to_remote` template filter function that has the ability to
map a role name to a remote. Attributes of the remote can then be
used to get the actual node ip or name.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit 361cbd4)
All `exec`-style function in teuthology appear to have a transformation
block that expands names like `all-roles` and `all-hosts`. With the new
cephadm.exec task that block appeared twice in cephadm.py. This change
removes the duplication by creating an _expand_roles function that
can be called from the command executing functions.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit bf1607a)
phlogistonjohn and others added 4 commits April 15, 2024 11:01
Looking at the code that expands `all-roles` and `all-hosts` there's no
proper error checking for when these values appear but there are >1
top-level roles in the task config. If a user does this it'll fail
but in a somewhat unclear manner. Add a new condition that raises a
clear exception in this case hopefully saving someone future debugging
time.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit 9670490)
Start a new subdir under cephadm suite for the new smb service
that cephadm can deploy. Add one new test that checks that a
smb service with domain membership can be deployed and connect
to it with smbclient from the samba client container image.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit 1f3001e)
To have the standalone (non-AD) server test function similarly to the AD
member server test we need to set a variable for samba client container
command similar to how the AD setup command does it.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit b2197e4)
Test minimal SMB deployment over CephFS, using local users (non-AD).
Upon successful deployment run minima smbclient command ('ls') to probe
Samba's share liveness.

Co-authored-by: John Mulligan <jmulligan@redhat.com>
Signed-off-by: Shachar Sharon <ssharon@redhat.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit 8bb5fb6)
@adk3798
Copy link
Contributor Author

adk3798 commented Apr 16, 2024

jenkins test make check

@adk3798
Copy link
Contributor Author

adk3798 commented Apr 16, 2024

https://pulpito.ceph.com/adking-2024-04-16_12:53:12-orch:cephadm-wip-adk2-testing-2024-04-15-1136-squid-distro-default-smithi/

Failures all fall under

  • in cluster log failures, known failures, still in the process of cleaning those up
  • mds_upgrade_sequence, known failure
  • staggered upgrade with agent enabled, known failure

Nothing to blocker merging

Copy link
Contributor

@phlogistonjohn phlogistonjohn left a comment

Choose a reason for hiding this comment

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

ok for backport

@adk3798 adk3798 merged commit 234b7f3 into ceph:squid Apr 16, 2024
11 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants