search

spiffe / helm-charts

Helm charts for SPIRE and other SPIFFE components.
Apache License 2.0
20 stars 22 forks source link

Add a test to ensure upgrades work #485

Open kfox1111 opened 1 year ago

kfox1111 commented 1 year ago

Did you know the ct tool has a flag to test upgrades?

We could add those too.

How does that work when we need to add additional, manual steps for upgrades? Guessing we may need some manual steps in some cases, like maybe the first time when we solve the crd update issue.

marcofranssen commented 12 months ago

https://github.com/helm/chart-testing/blob/main/ct/cmd/install.go#L64-L66 this flag could be set, not tested it though, I guess this compares against what is released vs the unreleased code. Could you try enabling that feature?

kfox1111 commented 12 months ago

Hmm.... It does look like it will do the right thing and not test if semver says it should be a breaking change. But, we are also pre 1.0, which means breaking changes could be made in a minor revision rather then at a major one? Will it do the right thing there?

kfox1111 commented 12 months ago

Can we merge this and continue to work on the ct upgrade tests in a different pr? I need the prod upgrade test to start working on crd upgrades for spire-controller-manager upgrades.

marcofranssen commented 12 months ago

Isn't this just duplicate testing if we just utilize ct?

kfox1111 commented 11 months ago

Isn't this just duplicate testing if we just utilize ct?

No, as it tests our production recommendation. we don't use ct on our production recommendation.

Also, could you please answer the question I asked?

marcofranssen commented 11 months ago

Isn't this just duplicate testing if we just utilize ct?

No, as it tests our production recommendation. we don't use ct on our production recommendation.

Also, could you please answer the question I asked?

Yes for sure we can just merge any PR and redo or change things in follow up PRs, however what I try to do here is to look beyond the horizon and see if all the test plumbing is the right thing to do if existing tools are there to do the same.

kfox1111 commented 11 months ago

Yes for sure we can just merge any PR and redo or change things in follow up PRs, however what I try to do here is to look beyond the horizon and see if all the test plumbing is the right thing to do if existing tools are there to do the same.

Yes, I understand what you are trying to do. I am looking beyond the horizon but I don't think your suggestion will work for what I know comes next, nor do I think we should hold up fixing the CRD issue while we try and get the perfect test system in place. We can iterate later.

So, what would you have me do on this pr? I need actionable feedback please. Scrap it and switch to ct, only for it to break irreparably when we try and implement the CRD fix isn't a good solution IMO.