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

Upgrade to latest version of Drone server #36712

Merged
merged 17 commits into from Sep 16, 2020
Merged

Conversation

sureshc
Copy link
Contributor

@sureshc sureshc commented Sep 11, 2020

Prerequisite for using Drone Autoscaler.

We were previously running Drone Server 1.0.0-rc.5 and Drone Worker/Agent 1.0.0-rc.5.
Upgrade to latest Drone Server (1.9.0) and latest Drone Runner (1.5.0), in preparation for adopting Drone Autoscaler and also to take advantage of new functionality in Drone.

Testing story

Drone build 17356 contains changes to dashboard, pegasus, lib, and shared folders to trigger unit tests for those sub-projects. It ran successfully, but I haven't analyzed the unit test logs closely to ensure that all of the expected unit tests were triggered correctly.

Drone build 17355 contains a change to the apps directory and failed during execution of unit tests. I'm still analyzing why they failed. I'm re-running a build on that commit.

Reviewer Checklist:

  • Tests provide adequate coverage
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

@sureshc sureshc requested review from wjordan and uponthesun and removed request for wjordan September 14, 2020 14:42
@@ -15,24 +16,6 @@ steps:
- git remote set-branches --add origin $DRONE_TARGET_BRANCH
- git fetch --depth 100 origin $DRONE_TARGET_BRANCH

- name: verify-pr
Copy link
Contributor Author

@sureshc sureshc Sep 14, 2020

Choose a reason for hiding this comment

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

Removing this logic because I saw this Step intermittently failing and because there's now a native feature in Drone server that provides this functionality. Here's a Pull Request from a fork that Drone correctly did not execute in a Build.

https://drone.cdn-code.org/code-dot-org/code-dot-org/settings
image

Choose a reason for hiding this comment

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

That's great, I never liked having so much logic in the .drone.yml file.

@@ -261,7 +261,8 @@ Resources:
KeyName: !Sub "winter-dev-${AWS::Region}"
SecurityGroupIds:
- !Ref DroneWorkerEcsSecurityGroup
ImageId: ami-0c4b05c0409735671
# ECS Optimized AMI - https://docs.aws.amazon.com/AmazonECS/latest/developerguide/ecs-optimized_AMI.html
ImageId: ami-04bb74f3ffa3aa3e2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Drone Server 1.9 issues Docker API calls that require a newer version of Docker, which is available by upgrading to the latest version of ECS-optimized (docker pre-installed) Amazon Linux.

Copy link

@uponthesun uponthesun left a comment

Choose a reason for hiding this comment

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

The changes look fine to me, I haven't been closely following the testing / rollout plan though. How's that going?

@@ -15,24 +16,6 @@ steps:
- git remote set-branches --add origin $DRONE_TARGET_BRANCH
- git fetch --depth 100 origin $DRONE_TARGET_BRANCH

- name: verify-pr

Choose a reason for hiding this comment

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

That's great, I never liked having so much logic in the .drone.yml file.

@sureshc sureshc merged commit bacb812 into staging Sep 16, 2020
@sureshc sureshc deleted the upgrade-drone-server branch September 16, 2020 18:23
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.

None yet

2 participants