DM-50020: USDF Rapid Analysis updates for LSSTCam#4497
Conversation
ctslater
force-pushed
the
tickets/DM-50020
branch
7 times, most recently
from
57677fc to
25f99f1
Compare
ctslater
force-pushed
the
tickets/DM-50020
branch
2 times, most recently
from
fe6baa4 to
c2f1c00
Compare
ctslater
force-pushed
the
tickets/DM-50020
branch
5 times, most recently
from
94c355c to
e62089c
Compare
ctslater
force-pushed
the
tickets/DM-50020
branch
from
ef67eef to
194efeb
Compare
ctslater
force-pushed
the
tickets/DM-50020
branch
from
b86ac01 to
6c94562
Compare
ctslater
force-pushed
the
tickets/DM-50020
branch
2 times, most recently
from
8f321d6 to
a548ba2
Compare
ctslater
force-pushed
the
tickets/DM-50020
branch
from
defeb3a to
5a9b37d
Compare
ctslater
force-pushed
the
tickets/DM-50020
branch
5 times, most recently
from
422dc2d to
5a9b37d
Compare
ctslater
force-pushed
the
tickets/DM-50020
branch
3 times, most recently
from
14089a8 to
e1f0b15
Compare
ctslater
force-pushed
the
tickets/DM-50020
branch
from
611f9e9 to
e1f0b15
Compare
ctslater
force-pushed
the
tickets/DM-50020
branch
from
cbe5e1a to
1fe5352
Compare
ctslater
force-pushed
the
tickets/DM-50020
branch
from
e14b817 to
359c670
Compare
sebastian-aranda
left a comment
sebastian-aranda
left a comment
There was a problem hiding this comment.
This looks great! I just found an issue with the values descriptions automatic generation so you can take a look 🙏 (and a couple doubts just in case).
| configMountpoint: [] | ||
| # -- This section holds the information necessary to claim persistent volumes. | ||
| # If the section is used, each object listed can have the following attributes defined: | ||
| # _name_ (The name of the persistent volume and the configMap), | ||
| # _items_ (Files in the configmap to mount), |
There was a problem hiding this comment.
I was a baffled by reading the README.md updates and seeing the configMountpoint and pvcMountpointClaim definitions/descriptions being swapped 😅 , it took me a bit to realize this definition was not properly positioned. Please swap lines 68 & 73 so the definitions are properly generated by the hooks 🙏 .
There was a problem hiding this comment.
Oh you're right, that is very confusing! I think I have it sorted out now.
| # This environment uses an ingress managed in a separate Kubernetes cluster, | ||
| # despite that configuration not being officially supported by Phalanx. | ||
| cert-manager: false | ||
| gafaelfawr: false |
There was a problem hiding this comment.
I guess this is an intentional change, but wanted to point it out just in case.
There was a problem hiding this comment.
Gafaelfawr was never actually enabled; this has been set to false since we first started using this cluster.
| repository: ghcr.io/lsst-sitcom/rubintv_production | ||
| tag: tickets-DM-50534-w48 | ||
| pullPolicy: IfNotPresent | ||
| location: USDF |
There was a problem hiding this comment.
Other values.yaml have the siteTag parameter as well, I guess that's not required here? (it is used only by templates/deployment.yaml)
There was a problem hiding this comment.
I had a look at siteTag and I don't see any reason to have both it and location. So I added a new commit that removes siteTag from deployment.yaml and from all the other values files, and I think it should all be fine? I looked at a diff of the generated yaml from helm with and without the change, and the only difference is the capitalization (which the python converts to lowercase anyways). I think this should all be safe, but does that seem right to you? Or I could make the change on a separate ticket.
There was a problem hiding this comment.
got it, sounds good 👍 .
ctslater
force-pushed
the
tickets/DM-50020
branch
from
359c670 to
556b7ba
Compare
ctslater
force-pushed
the
tickets/DM-50020
branch
from
556b7ba to
e40d04b
Compare
ctslater
force-pushed
the
tickets/DM-50020
branch
from
e40d04b to
39f1eb8
Compare
3 participants
This has been a very-long-lived branch, and most of the work has already been merged elsewhere but this cleans up the remaining pieces: