Skip to content
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

Fix Octopus.Client references for dotnet-script #1156

Merged
merged 9 commits into from
Nov 16, 2023

Conversation

IsaacCalligeros95
Copy link
Contributor

@IsaacCalligeros95 IsaacCalligeros95 commented Oct 10, 2023

There were several issues with the rollout of the dotnet-script update, namely the comms and lack of customer awareness. During this a few issues were raised:

  • References to Octopus.Client conflict with the Octopus class in the bootstrapper and throw errors when using it i.e. #r "nuget: Octopus.Client" using Octopus.Client;
  • Support for net7.0
  • net6.0 SDK requirement

This PR addresses some technical issues customers experienced during the attempted rollout. We are not addressing all of these issues.

References to Octopus.Client have been fixed however the Octopus class had to be removed from the C# Bootstrapper. This brings the C# scripting in line with Powershell, where calls like Octopus.SetVariable are now SetVariable and Octopus.Parameters are now OctopusParameters. This will have to be updated in the front end source code editor, and should reflect the environment variable toggle. This will also have to be documented.

This change still requires net6.0 on the deployment target, we can work around this using the Dotnet.Script Nuget package, copying across the program.cs and building self-contained versions of the project. This is ~160MB for the Windows build alone. This isn't a practical addition to Calamari considering the space taken. If customers cannot install net6.0 they can use the PowerShell ScriptCS workaround.

image

Similar to above we currently only support net6.0, this is due to the dotnet-tool bundle we include, which can be updated to include both net6.0 and net7.0, the size in this case isn't as much of an issue. However, we'd have to check the host dotnet version prior to execution, in initial discussions we'd like to avoid this. Feel free to push back on this point, it may be worth a more in-depth discussion. We'll need to keep this in mind for dotnet versioning in Calamari and dotnet-script this dependency isn't good.

This depends on the server changes in https://github.com/OctopusDeploy/OctopusDeploy/pull/21279
[sc-61383]

Comms:
OctopusDeploy/blog#1077
OctopusDeploy/docs#2035 - TBD on merging and calling out the multiple C# scripting engines for now.

Before merging this we will need to notify customers using the Dotnet Script functionality, I'll reach out to support in the dotnet-script channel.

After:

Scripts:
image
(TeamMembers updated for run)
image

Output:
image
image

Running on WSL and ubuntu containers in WSL
image

With Class based bootstrapper & dotnet-script:
DotnetScriptOctopusClassProcess
DotnetScriptOctopusClassProcess

No feature toggles running ScriptCS:
image

@IsaacCalligeros95 IsaacCalligeros95 force-pushed the isaac/dotnet-script-fixes branch from 6ab52a1 to e89de00 Compare October 11, 2023 00:36
@IsaacCalligeros95 IsaacCalligeros95 force-pushed the isaac/dotnet-script-fixes branch from e89de00 to 2a2e9dc Compare October 11, 2023 02:40
@IsaacCalligeros95 IsaacCalligeros95 requested a review from a team October 11, 2023 03:26
@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #61383: Dotnet-Script fixes.

@IsaacCalligeros95 IsaacCalligeros95 changed the title Dotnet Script fixes Fix Octopus.Client references for dotnet-script Oct 11, 2023
Copy link
Contributor

@hnrkndrssn hnrkndrssn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@IsaacCalligeros95
Copy link
Contributor Author

LGTM 👍

I've updated this to include a toggle for the existing class-based bootstrapper. This is for a specific customer that has already migrated and will prevent failing deployments until they are ready to migrate.
Related Server PR - https://github.com/OctopusDeploy/OctopusDeploy/pull/21279

Copy link
Contributor

@hnrkndrssn hnrkndrssn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants