masaori335 commented on code in PR #13191:
URL: https://github.com/apache/trafficserver/pull/13191#discussion_r3286826607
##########
src/iocore/cache/Store.cc:
##########
@@ -218,25 +220,96 @@ Span::~Span()
Result
Store::read_config()
{
- ats_scoped_str storage_path(RecConfigReadConfigPath(nullptr,
ts::filename::STORAGE));
+ config::StorageConfig parsed;
+
+ // Check storage.yaml, storage.config and volume.config
+ ats_scoped_str storage_yaml_path(RecConfigReadConfigPath(nullptr,
ts::filename::STORAGE_YAML));
+ const bool yaml_exists = storage_yaml_path &&
swoc::file::exists(swoc::file::path(storage_yaml_path.get()));
+
+ ats_scoped_str storage_config_path(RecConfigReadConfigPath(nullptr,
ts::filename::STORAGE));
+ const bool storage_config_exists = storage_config_path &&
swoc::file::exists(swoc::file::path(storage_config_path.get()));
- Note("%s loading ...", ts::filename::STORAGE);
+ ats_scoped_str
volume_config_path(RecConfigReadConfigPath("proxy.config.cache.volume_filename",
ts::filename::VOLUME));
+ const bool volume_config_exists = volume_config_path &&
swoc::file::exists(swoc::file::path(volume_config_path.get()));
config::StorageParser parser;
- auto parse_result = parser.parse(storage_path.get());
- if (!parse_result.ok()) {
- std::string msg;
- for (auto const &annotation : parse_result.errata) {
- if (!msg.empty()) {
- msg += "; ";
+
+ if (yaml_exists) {
+ Note("%s loading ...", ts::filename::STORAGE_YAML);
+
+ auto parse_result = parser.parse(storage_yaml_path.get());
+ if (!parse_result.ok()) {
+ std::string msg;
+ for (auto const &annotation : parse_result.errata) {
+ if (!msg.empty()) {
+ msg += "; ";
+ }
+ msg += std::string(annotation.text());
}
- msg += std::string(annotation.text());
+ return Result::failure("failed to load %s: %s",
ts::filename::STORAGE_YAML, msg.c_str());
}
- return Result::failure("failed to load %s: %s", ts::filename::STORAGE,
msg.c_str());
+ parsed = std::move(parse_result.value);
+
+ // If a legacy storage.config or volume.config also exists, warn that it
is being ignored.
+ if (storage_config_exists) {
+ Note("%s exists alongside %s; the legacy file is ignored. "
+ "To resolve, either: (a) migrate %s to %s (e.g. 'traffic_ctl config
convert storage') and remove %s, "
+ "or (b) remove %s to fall back to %s.",
+ ts::filename::STORAGE, ts::filename::STORAGE_YAML,
ts::filename::STORAGE, ts::filename::STORAGE_YAML,
+ ts::filename::STORAGE, ts::filename::STORAGE_YAML,
ts::filename::STORAGE);
+ }
+
+ if (volume_config_exists) {
+ Note("%s exists alongside %s; the legacy file is ignored. "
+ "To resolve, either: (a) migrate %s to %s (e.g. 'traffic_ctl config
convert storage') and remove %s, "
+ "or (b) remove %s to fall back to %s.",
+ ts::filename::VOLUME, ts::filename::STORAGE_YAML,
ts::filename::VOLUME, ts::filename::STORAGE_YAML, ts::filename::VOLUME,
+ ts::filename::STORAGE_YAML, ts::filename::VOLUME);
Review Comment:
That's true, but too much, I think.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
