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

Switch no-fork architecture #6

Merged
merged 2 commits into from
Jan 23, 2025
Merged

Conversation

turkenf
Copy link
Contributor

@turkenf turkenf commented Jan 16, 2025

Description of your changes

This PR switches provider-upjet-azapi to no-fork architecture.

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

https://github.com/upbound/provider-upjet-azapi/actions/runs/12831935067
https://github.com/upbound/provider-upjet-azapi/actions/runs/12884796491

Manually tested ResourceAction resource successfully and verified from Azure console:

NAME                                             SYNCED   READY   EXTERNAL-NAME
                                     AGE
resourceaction.resources.azapi.upbound.io/stop   True     True    /subscriptions/038f2b7c-3265-43b8-8624-c9ad5da610a8/resourceGroups/rg-for-azapi/providers/Microsoft.AppP
latform/Spring/example-spring/stop   6m1s

Screenshot 2025-01-22 at 00 34 12

@sergenyalcin sergenyalcin force-pushed the prep-no-fork branch 3 times, most recently from c843ac8 to 8023e19 Compare January 17, 2025 15:23
@sergenyalcin
Copy link
Member

/test-examples="examples/resources/v1beta1/resource.yaml"

Signed-off-by: Fatih Türken <[email protected]>
Signed-off-by: Sergen Yalçın <[email protected]>
@sergenyalcin
Copy link
Member

/test-examples="examples/resources/v1beta1/resource.yaml"

@sergenyalcin sergenyalcin changed the title [TEST] - Switch no-fork architecture Switch no-fork architecture Jan 21, 2025
@sergenyalcin sergenyalcin marked this pull request as ready for review January 21, 2025 10:26
Copy link
Contributor Author

@turkenf turkenf left a comment

Choose a reason for hiding this comment

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

Thank you for your work on this PR @sergenyalcin. Manually tested the ResourceAction resource and added it to the description. LGTM 🙌

Copy link

@mergenci mergenci left a comment

Choose a reason for hiding this comment

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

Thanks @turkenf and @sergenyalcin 🙏 I couldn't find any important issues, so I left a few nit comments 😅 The only issue I had on my local environment was with the go version specification. Please see my comments below.

go.mod Show resolved Hide resolved
cmd/provider/main.go Outdated Show resolved Hide resolved
cmd/provider/main.go Show resolved Hide resolved
cmd/provider/main.go Outdated Show resolved Hide resolved
@sergenyalcin
Copy link
Member

@mergenci I addressed your comments. I also realized that I haven't updated the Dockerfile. Since we can't fork cli and provider process anymore, a simpler Dockerfile is enough. I also committed this.

@sergenyalcin
Copy link
Member

/test-examples="examples/resources/v1beta1/resource.yaml"

@sergenyalcin sergenyalcin requested a review from mergenci January 23, 2025 09:25
Copy link

@mergenci mergenci left a comment

Choose a reason for hiding this comment

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

Good catch @sergenyalcin 💪 Let's go 🙌

@sergenyalcin sergenyalcin merged commit 7e5934c into upbound:main Jan 23, 2025
8 checks passed
@turkenf turkenf deleted the prep-no-fork branch January 23, 2025 10:14
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