-
Notifications
You must be signed in to change notification settings - Fork 120
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Autochannel feature #75
Conversation
Tweak expected step count to just be releasePlan.Steps.Count as the release plan already only includes steps that are package steps
…hannel couldnt be determined
In order to select the right channel based on the provided package versions, we need to simulate what it would happen if we created that release into each of the channels, and pick the right one based on which ReleasePlan was the most viable. This means we needed a way to reliably and consistently calculate the ReleasePlan, instead of doing it inline in the CreateReleaseCommand. This logic has been moved into the ReleasePlanBuilder. Now the CreateReleaseCommand can create all of the possible ReleasePlan combinations and select the most viable. I think overall this has simplified the code quite a lot, and consolidated a lot of the responsibilities into the building of ReleasePlan, instead of scattered around the code base. I've ended up removing these tests temporarily and will add tests to exercise the new shape.
throw new CommandException("Package versions could not be resolved for one or more of the package steps in this release. See the errors above for details. Either ensure the latest version of the package can be automatically resolved, or set the version to use specifically by using the --package argument."); | ||
} | ||
|
||
if (IgnoreIfAlreadyExists) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I raised this in my first PR but to me it seems this logic is out of whack? if IgnoreIfAlreadyExists == true
then we check if a release exists and throw an exception if it does. Shouldn't the if condition be if (!IgnoreIfAlreadyExists)
?
Conflicts: source/Octopus.Cli.Tests/Octopus.Cli.Tests.csproj source/Octopus.Cli/Octopus.Cli.csproj
No channel, no rules to fail!
After some offline discussion with @MJRichardson and @ryangribble we are leaning towards making the new
I certainly think this behaviour would be what I'd expect (least-surprise) if I was coming to channels in Octopus for the first time: just take the details I'm giving you and make the release work, otherwise tell me what's wrong and how to fix it. |
👍 I also think this is the way to go, as it means we dont need to update the "create release" step on all of our teamcity builds to start using channels on those octopus projects |
throw new CommandException($"At least one step violates the package version rules for the Channel '{plan.Channel.Name}'. Either correct the package versions for this release, select a different channel using --channel=MyChannel argument, let Octopus select the best channel using the --autoChannel argument, or ignore these version rules altogether by using the --ignoreChannelRules argument."); | ||
} | ||
|
||
if (IgnoreIfAlreadyExists) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just raising this one again... does this logic make sense?
if IgnoreIfAlreadyExists == true
and we do find an existing release, we exit without doing anything. Shouldn't it be continuing on and replacing that release?
The logic here is probably meant to say if (!IgnoreIfAlreadyExists)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I agree. I'll look into that today as part of this renovation. :) Thanks for the reminder.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, looking at this, I think what the flag actually means is "I'm just going to blindly create releases from my build server, and if I end up doing a duplicate build, and the release already exists, just ignore creating the release and don't get so upset"
I think it's easy to misinterpret, as I did at first glance. I'm going to leave that logic well alone to avoid breaking people who are already using this as a way to make their build servers quiet. :) Hope that's OK.
We had a big discussion in #75 about this. Made some tweaks to the logging Renamed Force to IgnoreChannelRules to make the meaning more clear
Conflicts: source/Octopus.Cli.Tests/Octopus.Cli.Tests.csproj source/Octopus.Cli/Commands/CreateReleaseCommand.cs source/Octopus.Cli/Octopus.Cli.csproj
Premise
Follows on from the good work done in #73. See there for more discussion on the "Why?" behind this feature.
Changes
The
CreateReleaseCommand
has historically been one big long transaction script, and was further built upon for Channels in Octopus 3.2. To make this feature rock-solid, we need to build aReleasePlan
per Channel, and figure out which ones are viable, and then make a good choice based on that information.ReleasePlan
a lot more central to the theme of creating a release.ReleasePlan
more consistentExploratory testing
A really good way to test this is using the Channels.Sample project from the Channels Walkthrough - or if you have access, just use the existing Channels.Sample project on the Demo server.
Example: Using
--whatif
to see what would happen by default (latest package version, no channel specified)Note: This would actually fail when posted to the server because the default channel would be used, which doesn't accept pre-releases. Is this good enough? Perhaps we can add some better information up-front, by assuming the default channel, and testing against that? Could be a compatibility risk though...
Example: Same, but hoping to select the right channel based on the latest available
Example: A release of
2.0.0-beta0001
using--autochannel