(PXP-6815): update MDS object after file upload. #29

johnfrancismccann · 2020-10-02T15:04:39Z

Extend indexs3client job to update the _bucket, _filename, _file_extension, and _upload_status keys in corresponding Metadata Service object after a file has been uploaded to the bucket.

Jira ticket

Metadata Service config must be nested in the CONFIG_FILE environment variable:

{
        "url": "http://indexd-service/index",
	"username": "mr happy cat",
	"password": "whiskers",
	"metadataService": {
		"url": "http://revproxy-service/mds",
		"username": "mr friendly cat",
		"password": "paws"
	}
}

~~PLEASE DO NOT MERGE YET. kube-setup-ssjdispatcher PR to set up MDS creds must be merged first.~~ Additionally, this indexs3client PR should go out in the same monthly release as the SSJDispatcher ticket/PR to require MDS creds on startup.

New Features

Update the _bucket, _filename, _file_extension, and _upload_status
keys in corresponding Metadata Service object after a file has been uploaded to the bucket.

Breaking Changes

Going forward, indexs3client now requires the Metadata Service to be deployed. Metadata Service creds are now required in the CONFIG_FILE environment variable in order for this job to run. Without both Indexd and MDS creds configured, this job will immediately exit and update neither Indexd or MDS.

Improvements

Determine if Indexd record already has hashes and size populated before downloading S3 object and calculating hashes. If so, don’t bother downloading/recalculating.
Define ConfigInfo struct so that the config (url and creds) for new services can be easily unmarshalled from the CONFIG_FILE environment variable
Make use of retryablehttp package to abstract http request retry logic and only retry on 500-range errors

Dependency updates

Add github.com/hashicorp/go-retryablehttp v0.6.7 to go.mod

Deployment changes

metadata-service must already be present or added to the "versions": { section of the cdis-manifest. After metadata-service has been confirmed to be in the cdis-manifest, the now-required Metadata Service creds for SSJDispatcher/indexs3client must be configured by running gen3 roll all with JENKINS_HOME not set. gen3 roll all calls gen3 kube-setup-ssjdispatcher, which now automatically configures the SSJDispatcher secret with the appropriate Metadata Service creds. After running gen3 roll all, SSJDispatcher should be configured such that SSJDispatcher can automatically pass the Metadata Service creds to the indexs3client job instance (i.e. after running gen3 roll all, no further action should be required by DevOps).

giangbui

LGTM!

paulineribeyre · 2020-10-02T20:33:15Z

handlers/handler.go

+
+	log.Printf("Attempting to get rev for record %s in Indexd", uuid)
+	rev, err := GetIndexdRecordRev(uuid, configInfo.Indexd.URL)
+	mdsUploadedBody := fmt.Sprintf(`{"_upload_status": "uploaded", "_filename": "%s"}`, filename)


the ticket and design doc mention _file_type but not _filename

I talked with @Avantol13 about this. We thought _filename could be preferable. I just updated the design doc to reflect this.

paulineribeyre · 2020-10-02T20:46:28Z

handlers/handler.go

+	}
+	log.Printf("Got rev %s from Indexd for record %s", rev, uuid)
+
+	updateMetadataObjectWrapper(uuid, configInfo, `{"_upload_status": "indexs3client job calculating hashes and size"}`)


all the others statuses in the design doc are a single word (or underscore-separated words)

you might want to check with @Avantol13 if this is an acceptable status. if not, maybe uploaded is fine, or maybe something like processing if you do want to differentiate it from uploaded?

Yeah just one thing to keep in mind is what _upload_status should be in the case that this job fails to download and calculate hashes for the file object (e.g. maybe the file is too big and can't be downloaded)

yeah, that's not detailed in the design doc, i'll let you and Alex make that call

paulineribeyre · 2020-10-02T20:57:15Z

handlers/metadata.go

+)
+
+// Checks for presence of MDS config, calls UpdateMetadataObject, and logs info
+func updateMetadataObjectWrapper(uuid string, configInfo *ConfigInfo, body string) {


this function doesn't exit in case of error, idk if that's on purpose

Yeah that was on purpose. The idea is that this wrapper does all the error handling and logging for the caller. I had been calling UpdateMetadataObject from handlers/handler.go in three places and just wanted to DRY.

paulineribeyre · 2020-10-02T21:03:37Z

.secrets.baseline

@@ -58,68 +58,241 @@
    }
  ],
  "results": {
-    "Gopkg.lock": [
+    "go.sum": [


you can exclude go.sum from the scanning

johnfrancismccann added 10 commits September 23, 2020 17:54

feat(update mds): put /mds/metadata/{did}

462fd0b

feat(update mds): handle errors, use retryablehttp

b57f40c

feat(update mds): use updateMetadataObjectWrapper

c75cd39

feat(update mds): handle empty Indexd creds

8ed7bfb

chore(.travis.yml): use Go version 1.14

ede6e48

test(getConfigInfo): assert creds are marshalled

e1acffa

style(update mds): remove commented code

0b83fb1

feat(update mds): read CONFIG_FILE before download

4b9944e

style(update mds): run go fmt ./handlers

88f34be

docs(mds): add function comments

1836be6

PlanXCyborg added the test-apis-dataUploadTest label Oct 2, 2020

johnfrancismccann requested review from giangbui, fantix, paulineribeyre and Avantol13 October 2, 2020 15:16

giangbui approved these changes Oct 2, 2020

View reviewed changes

paulineribeyre reviewed Oct 2, 2020

View reviewed changes

chore(.secrets.baseline): exclude go.sum

e27a4b0

themarcelor changed the title ~~(PXP-6815): update MDS object after file upload~~ (PXP-6815): update MDS object after file upload. Oct 4, 2020

johnfrancismccann added 3 commits October 8, 2020 00:43

feat(update mds): populate _file_extension

7c87976

feat(_upload_status): set to error before panic

4f03844

feat(update mds): require mds creds in CONFIG_FILE

7c38369

This was referenced Oct 13, 2020

(PXP-6861): configure Metadata Service creds in kube-setup-ssjdispatcher uc-cdis/cloud-automation#1428

Merged

(PXP-6977): make mds creds required for service startup uc-cdis/ssjdispatcher#32

Merged

giangbui approved these changes Oct 20, 2020

View reviewed changes

johnfrancismccann merged commit e9ca31f into master Oct 20, 2020

johnfrancismccann mentioned this pull request Oct 7, 2021

(PXP-8853): Explain why MDS 404 might be ok in logs #34

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(PXP-6815): update MDS object after file upload. #29

(PXP-6815): update MDS object after file upload. #29

johnfrancismccann commented Oct 2, 2020 •

edited

Loading

giangbui left a comment

paulineribeyre Oct 2, 2020

johnfrancismccann Oct 2, 2020

paulineribeyre Oct 2, 2020

johnfrancismccann Oct 2, 2020

paulineribeyre Oct 2, 2020

paulineribeyre Oct 2, 2020

johnfrancismccann Oct 2, 2020

paulineribeyre Oct 2, 2020

(PXP-6815): update MDS object after file upload. #29

(PXP-6815): update MDS object after file upload. #29

Conversation

johnfrancismccann commented Oct 2, 2020 • edited Loading

New Features

Breaking Changes

Improvements

Dependency updates

Deployment changes

giangbui left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnfrancismccann commented Oct 2, 2020 •

edited

Loading