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

Octopus Tentacle Image #80

Merged
merged 3 commits into from
Jul 10, 2024
Merged

Octopus Tentacle Image #80

merged 3 commits into from
Jul 10, 2024

Conversation

alismx
Copy link
Collaborator

@alismx alismx commented May 24, 2024

Related Issue

Changes Proposed

  • Creates an initial Tentacle Dockerfile that can be built through our GH actions.
  • The image will work as either:
    • Deployment Target: With Octopus Deploy, you can deploy software to Windows servers, Linux servers, Microsoft Azure, AWS, Kubernetes clusters, cloud regions, or an offline package drop. Regardless of where you’re deploying your software, these machines and services are known as your deployment targets.
    • Worker: Workers are machines that can execute tasks that don’t need to be run on the Octopus Server or individual deployment targets.

Additional Information

  • This page has useful information about Deployment target licensing
  • This page has information on how to run this image as a Deployment Target or a Worker.
  • Will need to create an API key
  • Seems like the internal port will need to be 10933
  • I'm not sure if setting TargetWorkerPool to a value that doesn't exist will create a pool or if it needs to be set to a value predefined in Octopus.

Testing

  • I built the image and did a test run and was able to register the tentacle with our OctopusDeploy instance.

Checklist for Primary Reviewer

Infrastructure

  • Consult the results of the terraform-plan job inside the "Terraform Checks" workflow run for this PR. Confirm that there are no unexpected changes!

Security

  • Changes with security implications have been approved by a security engineer (changes to authentication, encryption, handling of PII, etc.)
  • Any dependencies introduced have been vetted and discussed

Cloud

  • Oncall has been notified if this change is going in after-hours
  • If there are changes that cannot be tested locally, this has been deployed to our Azure dev environment for verification.

@alismx alismx marked this pull request as draft May 24, 2024 19:51
@alismx alismx marked this pull request as ready for review May 24, 2024 20:12
@alismx alismx requested a review from marycrawford June 10, 2024 18:56
@@ -0,0 +1,8 @@
FROM octopusdeploy/tentacle:8.1.1693
Copy link
Collaborator

Choose a reason for hiding this comment

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

Non-blocking: the newest version is 8.1.1838. These update pretty frequently, but I am also fine leaving it as-is for now. We'll have to come up with an upgrade strategy if/when we ever onboard STLT users to this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can push this update along with a name update related to your other comment

${{ inputs.acr_registry }}/dibbs-cloud/octopusdeploy:latest

- name: Build and push Docker image
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we modify this to indicate that it's the tentacle? Or something else to set it apart from the step outlined at line 34?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

Copy link
Collaborator

@shanice-skylight shanice-skylight left a comment

Choose a reason for hiding this comment

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

Reviewed the files and the terraform plan outputs. The only warning of interest was in the Prod output there are a fewWarning: Value for undeclared variable warnings. Are these intended to be future variables?

I don't see this as a must to change I was just interested in the intent of the variables. LGTM

@alismx
Copy link
Collaborator Author

alismx commented Jul 10, 2024

Reviewed the files and the terraform plan outputs. The only warning of interest was in the Prod output there are a fewWarning: Value for undeclared variable warnings. Are these intended to be future variables?

I don't see this as a must to change I was just interested in the intent of the variables. LGTM

I appreciate the call-out, with all the flux in this project, I think they're safe to ignore for now. I'm going to go ahead and merge this.

@alismx alismx merged commit 623ce25 into main Jul 10, 2024
7 checks passed
@alismx alismx deleted the alis/79 branch July 10, 2024 20:02
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.

3 participants