Update patch set 3
Patch Set 3:
(4 comments)
Patch-set: 3
Attention: {"person_ident":"Gerrit User 4393 \u003c4393@4a232e18-c5a9-48ee-94c0-e04e7cca6543\u003e","operation":"REMOVE","reason":"\u003cGERRIT_ACCOUNT_4393\u003e replied on the change"}
Attention: {"person_ident":"Gerrit User 9303 \u003c9303@4a232e18-c5a9-48ee-94c0-e04e7cca6543\u003e","operation":"ADD","reason":"\u003cGERRIT_ACCOUNT_4393\u003e replied on the change"}
This commit is contained in:
Gerrit User 4393
Gerrit Code Review
parent
2eeac230d1
commit
0edfac0f06
|
|
@ -69,6 +69,24 @@
|
|||
"revId": "52923f9fdde9e2e63e3bf667f30900ca8f7281f5",
|
||||
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
||||
},
|
||||
{
|
||||
"unresolved": true,
|
||||
"key": {
|
||||
"uuid": "df6286a7_fd1cec19",
|
||||
"filename": "specs/2024.2/approved/glance_store/improve-filesystem-driver.rst",
|
||||
"patchSetId": 3
|
||||
},
|
||||
"lineNbr": 36,
|
||||
"author": {
|
||||
"id": 4393
|
||||
},
|
||||
"writtenOn": "2024-05-10T14:00:23Z",
|
||||
"side": 1,
|
||||
"message": "Based on the answer(s) below, yes. If you go the file marker route, a very simple \"check for existence of file means healthy\" plugin added to oslo could make an easy thing for any service that has similar needs to glance.",
|
||||
"parentUuid": "01d8942d_fe4d3534",
|
||||
"revId": "52923f9fdde9e2e63e3bf667f30900ca8f7281f5",
|
||||
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
||||
},
|
||||
{
|
||||
"unresolved": true,
|
||||
"key": {
|
||||
|
|
@ -104,6 +122,24 @@
|
|||
"revId": "52923f9fdde9e2e63e3bf667f30900ca8f7281f5",
|
||||
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
||||
},
|
||||
{
|
||||
"unresolved": true,
|
||||
"key": {
|
||||
"uuid": "f35fb2d1_7bd1837b",
|
||||
"filename": "specs/2024.2/approved/glance_store/improve-filesystem-driver.rst",
|
||||
"patchSetId": 3
|
||||
},
|
||||
"lineNbr": 68,
|
||||
"author": {
|
||||
"id": 4393
|
||||
},
|
||||
"writtenOn": "2024-05-10T14:00:23Z",
|
||||
"side": 1,
|
||||
"message": "That\u0027s the desired behavior, not the current behavior, right?",
|
||||
"parentUuid": "0064c1a7_9eee1658",
|
||||
"revId": "52923f9fdde9e2e63e3bf667f30900ca8f7281f5",
|
||||
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
||||
},
|
||||
{
|
||||
"unresolved": true,
|
||||
"key": {
|
||||
|
|
@ -139,6 +175,24 @@
|
|||
"revId": "52923f9fdde9e2e63e3bf667f30900ca8f7281f5",
|
||||
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
||||
},
|
||||
{
|
||||
"unresolved": true,
|
||||
"key": {
|
||||
"uuid": "59c2f8f9_2f32b447",
|
||||
"filename": "specs/2024.2/approved/glance_store/improve-filesystem-driver.rst",
|
||||
"patchSetId": 3
|
||||
},
|
||||
"lineNbr": 77,
|
||||
"author": {
|
||||
"id": 4393
|
||||
},
|
||||
"writtenOn": "2024-05-10T14:00:23Z",
|
||||
"side": 1,
|
||||
"message": "Well, I don\u0027t really think you can *just* do either that or what you\u0027re proposing here. I think you need some sort of config from the user to know their intent. This seems to imply that it will always be started with the proper arrangement, but the container could just as easily be started without NFS available and then it will assume that no such mount is expected. There might even be a race between setup of the pod and the nfs mount and the glance startup check to determine the canonical disk config.\n\nThe reason containers don\u0027t *start* healthy automatically is so that they can survey the environment and then go from \"starting\" to \"healthy\" (or whatever the startup state is) after they determine that things are good. So I really think you need some \"intent\" from the operator other than just \"my dir was a mount when I started so that must be correct.\"\n\nI\u0027m not really sure what to suggest here, other than some config that says something very specific about the mount that we can check. Just checking \"ismount\" will probably return true for almost every container environment, since in almost all cases glance\u0027s images directory will at *least* be bind-mounted from the host.\n\nI think maybe something better might be a config knob that tells glance to look for some \"marker file\" in the images directory. Something (kinda like a tombstone) that should always be in the images directory to know it\u0027s the images directory. Something as simple as `/var/lib/glance/images/GLANCE` or `/var/lib/glance/images/.glance`. Then if that file is there (which much be provisioned the first time) we know we\u0027re looking at our images dir, regardless of where/how it\u0027s mounted.",
|
||||
"parentUuid": "e9ef2ffb_0b99be67",
|
||||
"revId": "52923f9fdde9e2e63e3bf667f30900ca8f7281f5",
|
||||
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
||||
},
|
||||
{
|
||||
"unresolved": true,
|
||||
"key": {
|
||||
|
|
@ -209,6 +263,30 @@
|
|||
},
|
||||
"revId": "52923f9fdde9e2e63e3bf667f30900ca8f7281f5",
|
||||
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
||||
},
|
||||
{
|
||||
"unresolved": true,
|
||||
"key": {
|
||||
"uuid": "546b033a_1e8d2af8",
|
||||
"filename": "specs/2024.2/approved/glance_store/improve-filesystem-driver.rst",
|
||||
"patchSetId": 3
|
||||
},
|
||||
"lineNbr": 102,
|
||||
"author": {
|
||||
"id": 4393
|
||||
},
|
||||
"writtenOn": "2024-05-10T14:00:23Z",
|
||||
"side": 1,
|
||||
"message": "Verify which behavior? The healthcheck is separate from the response code for actual image operations.\n\nIMHO, the way this should work is that if the mount disappears, the healthcheck goes to \"unhealthy\" and k8s stops sending requests to this worker.. it\u0027ll likely restart the pod (or some other remedy) and only once it goes back to healthy, start sending requests.\n\nA good reason not to return 4xx for image operations are where they might cause another service to do something wrong. Imagine if nova or cinder had some operation where a user deletes an instance (or volume) with a \"recurisive\" flag that means nova or cinder should also delete all snapshots in glance of that thing. Usually the way to do that is we list the images, then walk through and delete them. Any that return a 4xx (at least a 404 or 410) are ignored and don\u0027t trigger a failure because they must be \"already deleted.\" If glance is in a bad state and is returning these codes, it could cause nova to assume the snapshots are already deleted and continue deleting the instance, when it really should abort because it can\u0027t do the thing it was asked to do.\n\nAnother example might be some \"employee has left, delete all his data for legal/privacy reasons.\" If this glance pod happens to be in a bad state, the HR/legal automation script will think \"yep, all the private data has been securely deleted\" when it hasn\u0027t.",
|
||||
"parentUuid": "98e79966_b7a9d2cc",
|
||||
"range": {
|
||||
"startLine": 102,
|
||||
"startChar": 38,
|
||||
"endLine": 102,
|
||||
"endChar": 46
|
||||
},
|
||||
"revId": "52923f9fdde9e2e63e3bf667f30900ca8f7281f5",
|
||||
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
||||
}
|
||||
]
|
||||
}
|
||||
Loading…
Reference in New Issue