Improve error message advice by including missing package names

Add additional checks for plugins that can depend on configuration.

Author: bbockelm

xrootd/plugin_check.go

@@ -387,41 +387,93 @@ func checkPluginExists(pluginName string, includeClientPaths bool) bool {
 	return false
 }
 
+// pluginPackageMap maps plugin base names (without extension) to the RPM package that provides them.
+// These were determined from an OSG EL9 environment; names may differ in other distros but
+// should be correct for the primary Pelican deployment targets.
+var pluginPackageMap = map[string]string{
+	"libXrdHttpPelican": "xrdhttp-pelican",
+	"libXrdS3":          "xrootd-s3-http",
+	"libXrdClPelican":   "xrdcl-pelican",
+	"libXrdThrottle":    "xrootd-server-libs",
+	"libXrdMacaroons":   "xrootd-server-libs",
+	"libXrdMultiuser":   "xrootd-multiuser",
+	"libXrdOssPosc":     "xrootd-s3-http",
+	"libXrdHTTPServer":  "xrootd-s3-http",
+	"libXrdOssGlobus":   "xrootd-s3-http",
+	"libXrdPurgeLotMan": "xrootd-lotman",
+}
+
+// pluginBaseName strips the extension from a plugin filename (e.g. "libXrdS3.so" -> "libXrdS3").
+func pluginBaseName(name string) string {
+	for _, ext := range []string{".so", ".dylib"} {
+		name = strings.TrimSuffix(name, ext)
+	}
+	return name
+}
+
+// checkAndAppendMissing checks if a plugin exists and appends it to the missing list if not found.
+func checkAndAppendMissing(missingPlugins *[]string, pluginName string, includeClientPaths bool) {
+	if !checkPluginExists(pluginName, includeClientPaths) {
+		*missingPlugins = append(*missingPlugins, pluginName)
+	}
+}
+
 // ValidateRequiredPlugins checks for the existence of required XRootD plugins based on the configuration.
 // It returns an error with a detailed message if any required plugin is missing.
 func ValidateRequiredPlugins(isOrigin bool, xrdConfig *XrootdConfig) error {
 	missingPlugins := []string{}
 
 	if isOrigin {
-		// Check for libXrdHttpPelican if drop privileges is enabled
+		// libXrdHttpPelican is required when drop privileges is enabled
 		if xrdConfig.Server.DropPrivileges {
-			if !checkPluginExists("libXrdHttpPelican.so", false) {
-				missingPlugins = append(missingPlugins, "libXrdHttpPelican.so")
-			}
+			checkAndAppendMissing(&missingPlugins, "libXrdHttpPelican.so", false)
+		}
+
+		// libXrdMacaroons is needed when macaroons are enabled
+		if xrdConfig.Origin.EnableMacaroons {
+			checkAndAppendMissing(&missingPlugins, "libXrdMacaroons.so", false)
+		}
+
+		// libXrdThrottle is needed when concurrency is configured
+		if xrdConfig.Origin.Concurrency > 0 {
+			checkAndAppendMissing(&missingPlugins, "libXrdThrottle.so", false)
 		}
 
-		// Check for libXrdS3 if using S3 storage type
-		if xrdConfig.Origin.StorageType == "s3" {
-			if !checkPluginExists("libXrdS3.so", false) {
-				missingPlugins = append(missingPlugins, "libXrdS3.so")
+		switch xrdConfig.Origin.StorageType {
+		case "posix":
+			// libXrdMultiuser is needed for multiuser mode
+			if xrdConfig.Origin.Multiuser {
+				checkAndAppendMissing(&missingPlugins, "libXrdMultiuser.so", false)
 			}
+			// libXrdOssPosc is needed for atomic uploads
+			if xrdConfig.Origin.EnableAtomicUploads {
+				checkAndAppendMissing(&missingPlugins, "libXrdOssPosc.so", false)
+			}
+		case "s3":
+			checkAndAppendMissing(&missingPlugins, "libXrdS3.so", false)
+		case "https":
+			checkAndAppendMissing(&missingPlugins, "libXrdHTTPServer.so", false)
+		case "globus":
+			checkAndAppendMissing(&missingPlugins, "libXrdHTTPServer.so", false)
+			checkAndAppendMissing(&missingPlugins, "libXrdOssGlobus.so", false)
 		}
 	} else {
 		// Cache-specific checks
-		// Check for libXrdHttpPelican if drop privileges is enabled
-		if xrdConfig.Server.DropPrivileges {
-			if !checkPluginExists("libXrdHttpPelican.so", false) {
-				missingPlugins = append(missingPlugins, "libXrdHttpPelican.so")
-			}
-		}
+		// libXrdHttpPelican is always required for caches (unconditional in cache template)
+		checkAndAppendMissing(&missingPlugins, "libXrdHttpPelican.so", false)
 
-		// Check for client plugin - this is needed for cache servers to support pelican:// URLs.
-		// The cache configuration writes client plugin settings to the cache-client.plugins.d directory
-		// (see CheckCacheEnv in xrootd_config.go), which configures XRootD to use libXrdClPelican.so
-		// for handling pelican:// protocol requests.
+		// libXrdClPelican is needed for cache servers to support pelican:// URLs.
 		// Include client plugin configuration paths for this check.
-		if !checkPluginExists("libXrdClPelican.so", true) {
-			missingPlugins = append(missingPlugins, "libXrdClPelican.so")
+		checkAndAppendMissing(&missingPlugins, "libXrdClPelican.so", true)
+
+		// libXrdPurgeLotMan is needed when Lotman is enabled
+		if xrdConfig.Cache.LotmanCfg.Enabled {
+			checkAndAppendMissing(&missingPlugins, "libXrdPurgeLotMan.so", false)
+		}
+
+		// libXrdThrottle is needed when concurrency is configured
+		if xrdConfig.Cache.Concurrency > 0 {
+			checkAndAppendMissing(&missingPlugins, "libXrdThrottle.so", false)
 		}
 	}
 
@@ -431,14 +483,34 @@ func ValidateRequiredPlugins(isOrigin bool, xrdConfig *XrootdConfig) error {
 		if runtime.GOOS == "darwin" {
 			envVars = "DYLD_LIBRARY_PATH, or DYLD_FALLBACK_LIBRARY_PATH"
 		}
+
+		// Build per-plugin install advice, deduplicating packages
+		seenPkgs := make(map[string]bool)
+		var pkgAdvice []string
+		for _, plugin := range missingPlugins {
+			base := pluginBaseName(plugin)
+			if pkg, ok := pluginPackageMap[base]; ok && !seenPkgs[pkg] {
+				seenPkgs[pkg] = true
+				pkgAdvice = append(pkgAdvice, pkg)
+			}
+		}
+
+		var installHint string
+		if len(pkgAdvice) > 0 {
+			installHint = "\nThe missing plugin(s) are provided by the following package(s): " +
+				strings.Join(pkgAdvice, ", ")
+		}
+
 		return errors.Errorf(
 			"Required XRootD plugin(s) not found: %s\n"+
 				"Please install the missing plugin(s) in one of the following directories:\n  %s\n"+
-				"Or set the %s environment variable to include the plugin location.\n"+
-				"Note: XRootD may add a version suffix to plugin names (e.g., libXrdHttpPelican-5.so for XRootD 5.x)",
+				"Or set the %s environment variable to include the plugin location.%s\n"+
+				"When searching on disk, XRootD version suffixes are automatically added to the filename\n"+
+				"(e.g., libXrdHttpPelican.so in config is expected to be libXrdHttpPelican-5.so on disk)",
 			strings.Join(missingPlugins, ", "),
 			strings.Join(searchPaths, "\n  "),
 			envVars,
+			installHint,
 		)
 	}
 

xrootd/plugin_check_test.go

@@ -403,23 +403,146 @@ func TestValidateRequiredPlugins(t *testing.T) {
 		}
 	})
 
-	t.Run("CacheWithDropPrivileges", func(t *testing.T) {
+	t.Run("OriginWithHttpsStorage", func(t *testing.T) {
+		xrdConfig := &XrootdConfig{
+			Origin: OriginConfig{
+				StorageType: "https",
+			},
+		}
+
+		err := ValidateRequiredPlugins(true, xrdConfig)
+		if err != nil {
+			assert.Contains(t, err.Error(), "libXrdHTTPServer.so")
+		}
+	})
+
+	t.Run("OriginWithGlobusStorage", func(t *testing.T) {
+		xrdConfig := &XrootdConfig{
+			Origin: OriginConfig{
+				StorageType: "globus",
+			},
+		}
+
+		err := ValidateRequiredPlugins(true, xrdConfig)
+		if err != nil {
+			assert.Contains(t, err.Error(), "libXrdHTTPServer.so")
+			assert.Contains(t, err.Error(), "libXrdOssGlobus.so")
+		}
+	})
+
+	t.Run("OriginWithMacaroons", func(t *testing.T) {
+		xrdConfig := &XrootdConfig{
+			Origin: OriginConfig{
+				StorageType:     "posix",
+				EnableMacaroons: true,
+			},
+		}
+
+		err := ValidateRequiredPlugins(true, xrdConfig)
+		if err != nil {
+			assert.Contains(t, err.Error(), "libXrdMacaroons.so")
+		}
+	})
+
+	t.Run("OriginWithConcurrency", func(t *testing.T) {
+		xrdConfig := &XrootdConfig{
+			Origin: OriginConfig{
+				StorageType: "posix",
+				Concurrency: 4,
+			},
+		}
+
+		err := ValidateRequiredPlugins(true, xrdConfig)
+		if err != nil {
+			assert.Contains(t, err.Error(), "libXrdThrottle.so")
+		}
+	})
+
+	t.Run("OriginWithMultiuser", func(t *testing.T) {
+		xrdConfig := &XrootdConfig{
+			Origin: OriginConfig{
+				StorageType: "posix",
+				Multiuser:   true,
+			},
+		}
+
+		err := ValidateRequiredPlugins(true, xrdConfig)
+		if err != nil {
+			assert.Contains(t, err.Error(), "libXrdMultiuser.so")
+		}
+	})
+
+	t.Run("OriginWithAtomicUploads", func(t *testing.T) {
+		xrdConfig := &XrootdConfig{
+			Origin: OriginConfig{
+				StorageType:         "posix",
+				EnableAtomicUploads: true,
+			},
+		}
+
+		err := ValidateRequiredPlugins(true, xrdConfig)
+		if err != nil {
+			assert.Contains(t, err.Error(), "libXrdOssPosc.so")
+		}
+	})
+
+	t.Run("CacheAlwaysRequiresHttpPelican", func(t *testing.T) {
+		// libXrdHttpPelican should always be checked for caches,
+		// regardless of DropPrivileges setting
 		xrdConfig := &XrootdConfig{
 			Server: ServerConfig{
-				DropPrivileges: true,
+				DropPrivileges: false,
 			},
 			Cache: CacheConfig{},
 		}
 
 		err := ValidateRequiredPlugins(false, xrdConfig)
-		// This may fail if plugins are not installed, which is expected
 		if err != nil {
 			assert.Contains(t, err.Error(), "Required XRootD plugin(s) not found")
-			// Should check for both libXrdHttpPelican and libXrdClPelican
-			assert.True(t,
-				strings.Contains(err.Error(), "libXrdHttpPelican.so") ||
-					strings.Contains(err.Error(), "libXrdClPelican.so"),
-			)
+			assert.Contains(t, err.Error(), "libXrdHttpPelican.so")
+		}
+	})
+
+	t.Run("CacheWithLotman", func(t *testing.T) {
+		xrdConfig := &XrootdConfig{
+			Cache: CacheConfig{
+				LotmanCfg: LotmanCfg{
+					Enabled: true,
+				},
+			},
+		}
+
+		err := ValidateRequiredPlugins(false, xrdConfig)
+		if err != nil {
+			assert.Contains(t, err.Error(), "libXrdPurgeLotMan.so")
+		}
+	})
+
+	t.Run("CacheWithConcurrency", func(t *testing.T) {
+		xrdConfig := &XrootdConfig{
+			Cache: CacheConfig{
+				Concurrency: 4,
+			},
+		}
+
+		err := ValidateRequiredPlugins(false, xrdConfig)
+		if err != nil {
+			assert.Contains(t, err.Error(), "libXrdThrottle.so")
+		}
+	})
+
+	t.Run("ErrorMessageFormat", func(t *testing.T) {
+		xrdConfig := &XrootdConfig{
+			Cache: CacheConfig{},
+		}
+
+		err := ValidateRequiredPlugins(false, xrdConfig)
+		if err != nil {
+			errMsg := err.Error()
+			// Should mention the specific packages that provide the plugins
+			assert.Contains(t, errMsg, "xrdhttp-pelican")
+			assert.Contains(t, errMsg, "xrdcl-pelican")
+			assert.NotContains(t, errMsg, "Note:")
 		}
 	})
 }