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

Remove more dapper #670

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mallardduck
Copy link
Member

@mallardduck mallardduck commented Jul 11, 2024

Types of Change

Removes dapper from GHA workflows by calling the scripts directly.

Linked Issues

Follow up for:
#668
#669

Will fix: #666

Additional Notes

@mallardduck mallardduck requested a review from a team as a code owner July 11, 2024 19:29
brandond
brandond previously approved these changes Jul 11, 2024
@brandond
Copy link
Member

I still see a drone check on this PR, does it need to be rebased?

@mallardduck
Copy link
Member Author

@brandond - yeah that's odd, seems like the repo settings may have had Drone PR added back?

@brandond
Copy link
Member

It's at least not blocking the merges any more, so I can go ahead and merge this if you'd like.

@mallardduck
Copy link
Member Author

I'll let @thardeck give this a review first and then after that lets merge it. I like the collaboration we're doing and think having them look it over and giving a review too will be good.

@thardeck
Copy link
Contributor

I still see a drone check on this PR, does it need to be rebased?

If I click on the branch of this pr it still shows the .drone.yml.

Copy link
Contributor

@thardeck thardeck left a comment

Choose a reason for hiding this comment

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

I would adapt the README.md examples to reflect the removal of the Makefile.

Besides that looks good to me.

@mallardduck
Copy link
Member Author

Ahh - yeah I was still on a stale master branch. Looks like I had never added an upstream to pull in the new changes and was on my out of sync fork still. Fixed that now and will adjust readme as mentioned.

I'm also looking into the add-tag job issue and will probably make a new PR for that. I will also look into addressing a way to clean up the gha- branches via workflow too, since I'm seeing a lot of those on the repo.

@mallardduck
Copy link
Member Author

So it seems our failing add-tag-to-existing-image like: https://github.com/rancher/image-mirror/actions/runs/9905573716/job/27365382388

Are being caused by an upstream k8s issue caused by an upstream (to them) issue with microsoft base images they use; based on this issue: kubernetes-csi/livenessprobe#273

Also just noticed the release note says they cannot build windows images; confused why this caused them to not publish linux images, but likely related.

@mallardduck
Copy link
Member Author

Wonder if we should merge this PR or close it? I know some folks still like the uniformity Dapper provides, so maybe we don't remove dapper? I've enjoyed keeping it around in a repo where I need to run the CI scripts locally. I'm not sure if that applies to this repo though, so maybe removing it is fine since we don't need that local use case?

@brandond
Copy link
Member

brandond commented Aug 2, 2024

I am +1 on dropping it from the workflows, but I do think there is value in leaving artifacts in the repo for devs that still use dapper for local testing - at least until we receive direction from management that we are to do away with it entirely.

@mallardduck
Copy link
Member Author

@brandond - this should be good to review now, scoped down to just remove dapper from CI running. However it remains for local dev usage.

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.

Drone Publish Jobs not Running
3 participants