Skip to content

Add Support for Scraper resource in amp provider - #1918

Merged
erhancagirici merged 3 commits into
crossplane-contrib:mainfrom
nujragan93:amp-scraper-support
Dec 1, 2025
Merged

Add Support for Scraper resource in amp provider#1918
erhancagirici merged 3 commits into
crossplane-contrib:mainfrom
nujragan93:amp-scraper-support

Conversation

@nujragan93

Copy link
Copy Markdown

Description of your changes

Add support for AMP Prometheus Scraper resource

Fixes #

I have:

  • Read and followed Crossplane's contribution process.
  • Run make generate and committed the results (ideally in a separate commit).
  • Not made any manual changes to generated files, and verified this with make check-diff.

How has this code been tested

  • Manual Testing Running AMP Provider with crossplane

  • Tested creating, observing and deletion of the resource

  • Tested referencing workspace object from scraper resource

Screenshot 2025-11-14 at 9 03 56 AM

@jeanduplessis

Copy link
Copy Markdown
Collaborator

/test-examples="examples/amp/cluster/v1beta1/scraper.yaml"

@nujragan93

nujragan93 commented Nov 21, 2025

Copy link
Copy Markdown
Author

@jeanduplessis looks like the e2e failed because the scraper was in a Creating state, I see that the external name of the scraper id was flipping between two IDs, I cant seem to understand the issue causing this, any help is appreciated

@erhancagirici erhancagirici left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@nujragan93 thanks for the PR! Looks good overall, left some minor change requests.

I checked this and was able to reproduce the issue. This looks like to be an issue at upjet causing an superfluous diff, which I am investigating with a potential fix. I'll further update here. For that no action is needed from your side.

Comment thread config/cluster/amp/config.go
Comment thread config/cluster/provider.go
@nujragan93

Copy link
Copy Markdown
Author

@erhancagirici Thanks for your review, Added amp config to namespaced type.

@erhancagirici

Copy link
Copy Markdown
Collaborator

/test-examples="examples/amp/cluster/v1beta1/scraper.yaml"

@erhancagirici

Copy link
Copy Markdown
Collaborator

After consuming the fix at Upjet, the Uptest run passed. Linter is complaining about some deprecated field usage (unrelated to this PR) after upjet bump, so opened a dedicated PR #1931 to handle the upjet bump. Will rebase this PR after that one is merged.

Nagarjun Krishnan added 3 commits December 1, 2025 13:24
Signed-off-by: Nagarjun Krishnan <nkrishnan@ancestry.com>
Signed-off-by: Nagarjun Krishnan <nkrishnan@ancestry.com>
Signed-off-by: Nagarjun Krishnan <nkrishnan@ancestry.com>
@erhancagirici

Copy link
Copy Markdown
Collaborator

/test-examples="examples/amp/cluster/v1beta1/scraper.yaml"

@erhancagirici erhancagirici left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks @nujragan93 for the contribution 🙌 ! I've rebased the PR to latest main with the fix included, and now it passes the tests. LGTM!

@erhancagirici
erhancagirici merged commit 6be693e into crossplane-contrib:main Dec 1, 2025
9 checks passed
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