Arvados

Should include a note in the upgrade notes.

Updates at 451fd7e - branch 19582-aws-s3v2-driver
Test run: developer-run-tests: #3317

• Enables Volumes.x.DriverParameters.UseAWSS3v2Driver by default.
• Updates upgrade notes explaining how to go back to the old driver if needed.

Lucas Di Pentima wrote in #note-4:

Updates at 451fd7e - branch 19582-aws-s3v2-driver
Test run: developer-run-tests: #3317

• Enables Volumes.x.DriverParameters.UseAWSS3v2Driver by default.
• Updates upgrade notes explaining how to go back to the old driver if needed.

DriverParameters is under SAMPLE in the Volumes section, it's not totally clear to me that those are actually taken as defaults? I don't know where in the code it would merge those. Did you confirm that changing it in config.default.yml actually changes the value in "arvados-server config-dump"?

I created a dummy volume in the config and used config-dump, the only key that's appearing is the one I defined.

      x94bf-nyw5e-222222222222222:
        AccessViaHosts:
          http://localhost:25108/:
            ReadOnly: false
        Driver: S3
        DriverParameters:
          IAMRole: abcd
        ReadOnly: false
        Replication: 0
        StorageClasses:
          default: true

It looks to me like the real actual defaults are applied here in S3AWSVolume.check()

    if v.ConnectTimeout == 0 {
        v.ConnectTimeout = s3DefaultConnectTimeout
    }
    if v.ReadTimeout == 0 {
        v.ReadTimeout = s3DefaultReadTimeout
    }

The parameters get loaded in chooseS3VolumeDriver using json.Unmarshal(volume.DriverParameters, v)

So for toggling the default value of this feature specifically, we either want to do something like: if the parameter isn't defined in DriverParameters (which is a json.RawMessage), then enable it.

On the other hand, a more general solution would be have the config system load default values in this case.

Updates at 5c0d99ac1
Test run: developer-run-tests: #3322

• Adds relevant code to chooseS3VolumeDriver() so that it applies the new default.
• I'm not sure how to test this, as it doesn't seem to be previously existing tests for the driver choosing mechanism.
• arvados-server config-dump won't show the expected output because the DriverParameters complete struct is realized deep in keepstore's code instead of the config loader.

Lucas Di Pentima wrote in #note-7:

Updates at 5c0d99ac1
Test run: developer-run-tests: #3322

• Adds relevant code to chooseS3VolumeDriver() so that it applies the new default.
• I'm not sure how to test this, as it doesn't seem to be previously existing tests for the driver choosing mechanism.
• arvados-server config-dump won't show the expected output because the DriverParameters complete struct is realized deep in keepstore's code instead of the config loader.

From chat: how about adding a log statement at startup that says which driver it is using, then we can quickly confirm that it is using the intended one.

Just to be clear, the branch looks fine, but I think it would be valuable to add a logging statement about which driver it is actually using and then manually confirming it is using the right one.

Sorry for the delay on this.

I've merged the latest main at 07baa0ed0 and built a dev version of the arvados-server binary. Then, copied it to keep0.tordo and added the SystemLogs.LogLevel=debug configuration.

Running the dev version shows that it's using the new S3 driver versus the goamz driver that the currently installed binary uses:

root@keep0:/home/lucas# ./arvados-server keepstore
{"ClusterID":"tordo","PID":9052,"level":"info","msg":"keepstore dev (go1.17.7) starting, pid 9052","time":"2022-10-31T19:50:08.541313775Z"}
{"ClusterID":"tordo","PID":9052,"level":"debug","msg":"Using AWS S3 v2 driver","time":"2022-10-31T19:50:08.541422667Z"}
{"ClusterID":"tordo","PID":9052,"level":"info","msg":"started volume tordo-nyw5e-000000000000000 (s3-bucket:\"tordo-nyw5e-000000000000000-volume\"), ReadOnly=false","time":"2022-10-31T19:50:08.541522053Z"}
{"ClusterID":"tordo","Listen":"10.253.255.42:25107","PID":9052,"Service":"keepstore","URL":"http://keep0.tordo.arvadosapi.com:25107/","Version":"dev (go1.17.7)","level":"info","msg":"listening","time":"2022-10-31T19:50:08.546093010Z"}
^C
root@keep0:/home/lucas# /usr/bin/keepstore
{"ClusterID":"tordo","PID":9057,"level":"info","msg":"keepstore 2.5.0~dev20221031171231 (go1.17.7) starting, pid 9057","time":"2022-10-31T19:50:31.027144607Z"}
{"ClusterID":"tordo","PID":9057,"level":"debug","msg":"Using goamz S3 driver","time":"2022-10-31T19:50:31.027250897Z"}
{"ClusterID":"tordo","PID":9057,"level":"info","msg":"started volume tordo-nyw5e-000000000000000 (s3-bucket:\"tordo-nyw5e-000000000000000-volume\"), ReadOnly=false","time":"2022-10-31T19:50:31.027288835Z"}
{"ClusterID":"tordo","Listen":"10.253.255.42:25107","PID":9057,"Service":"keepstore","URL":"http://keep0.tordo.arvadosapi.com:25107/","Version":"2.5.0~dev20221031171231 (go1.17.7)","level":"info","msg":"listening","time":"2022-10-31T19:50:31.033830766Z"}

Running tests just in case: developer-run-tests: #3348

Lucas Di Pentima wrote in #note-11:

Sorry for the delay on this.

I've merged the latest main at 07baa0ed0 and built a dev version of the arvados-server binary. Then, copied it to keep0.tordo and added the SystemLogs.LogLevel=debug configuration.

Running the dev version shows that it's using the new S3 driver versus the goamz driver that the currently installed binary uses:

[...]

Perfect, so it was already logging the driver, just needed to verify it. This LGTM.