krane icon indicating copy to clipboard operation
krane copied to clipboard
verified publisher icon Shopify

fix: initial commit for deploy fix when having duplicate kinds

Open renescheepers opened this issue 3 years ago • 4 comments

  • Dropped support for Ruby 2.7, has been EOL since 2019-12-25
  • Dropped support for Kubernetes 1.19, has been EOL since 2021-10-28
  • Fixed #895

renescheepers avatar Jul 13 '22 12:07 renescheepers

I need to go through the tests, but I'm on board with the solution in general.

At first I was apprehensive about explicitly specifying groups for the hardcoded resources, but on the whole I don't think it's a problem:

  • They represent mature objects that are unlikely to radically change
  • If they do change, we can simply define a new GroupKind and ride out the deprecation

If the group kind for an existing resources changes we could simply copy the class and extend the original one.

It does tie our implementation to a specific set of kubernetes revisions, but as we already maintain a fairly tight skew, I don't see that as very problematic, overall.

This is already the case at the moment.

renescheepers avatar Jul 25 '22 09:07 renescheepers

FYI: You will need to merge with this PR: https://github.com/Shopify/krane/pull/900 Version 2.4.9 will be cut from that, shortly.

jpfourny avatar Jul 27 '22 17:07 jpfourny

fix: all tests serial

I don't see this actually changing tests to run differently, but the commit message caught my eye. It's important that Krane be able to run multiple deploys currently, even when used as a gem. So just wanted to make super sure we are aware of that. 🙏

KnVerey avatar Aug 16 '22 22:08 KnVerey

fix: all tests serial

I don't see this actually changing tests to run differently, but the commit message caught my eye. It's important that Krane be able to run multiple deploys currently, even when used as a gem. So just wanted to make super sure we are aware of that. 🙏

Yes, because the tests take a long time I went through each test-suite separately. One of those is integration-serial so while I didn't specifically touch any files there, I needed to do these fixes to have these tests pass.

renescheepers avatar Aug 17 '22 12:08 renescheepers

Not planning on merging this pull request. This pull request was needed to resolve some issues that would happen when used in combination with Crossplane. However we stopped using Crossplane. Even though it still would be nice to fix, it is way too risky to deploy since this is used everywhere.

renescheepers avatar Nov 30 '22 15:11 renescheepers