Skip to content

Conversation

@nishaatr
Copy link
Contributor

@nishaatr nishaatr commented Nov 11, 2024

This came about at the back of removing Code Samples from Platform releases https://hazelcast.atlassian.net/browse/DI-313

TODO (need help here)

  1. Sort out HZ_LICENSEKEY. We have these in GH so not sure which one to use
    • HAZELCAST_ENTERPRISE_KEY (may be start with this?)
    • HAZELCAST_ENTERPRISE_KEY_V7 (this will future proof perhaps?)
  2. Limit who runs this via the action (checking for org membership) or via the GitHub repo config?
  3. Remove -DskipTests once above is sorted

Fixes: DI-336

@nishaatr nishaatr changed the title Di 336 migrate jenkins pr builder Migrate Jenkins PR builder to GitHub Action [DI-336] Nov 11, 2024
@nishaatr nishaatr requested a review from JackPGreen November 11, 2024 10:11
@JackPGreen
Copy link
Contributor

  1. Sort out HZ_LICENSEKEY. We have these in GH so not sure which one to use

    • HAZELCAST_ENTERPRISE_KEY (may be start with this?)
    • HAZELCAST_ENTERPRISE_KEY_V7 (this will future proof perhaps?)

Ideally V7 unless we need backwards compatibility support - https://hazelcast.slack.com/archives/C01JU7ZJYGP/p1719232648844259

  1. Limit who runs this via the action (checking for org membership) or via the GitHub repo config?

Typically (at least on open-source non-hazelcast repos) this is done via GitHub Repo config, but some Hazelcast repos use hazelcast-tpm/membership.

Copy link
Contributor

@JackPGreen JackPGreen left a comment

Choose a reason for hiding this comment

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

We also need to log that the Jenkins build should be disabled.

@nishaatr
Copy link
Contributor Author

Typically (at least on open-source non-hazelcast repos) this is done via GitHub Repo config, but some Hazelcast repos use hazelcast-tpm/membership.

Fixed 967b2d882eae
Pls check?

- name: Detect untrusted community PR
if: ${{ needs.check_for_membership.outputs.check-result == 'false' }}
run: |
gh pr comment ${{ github.event.number }} \
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not actually sure we want a comment here, an annotation the job is probably enough.

(I know you copied it from the PR builder, which does that - but I deliberately used comments there for backwards compatibility reasons)

Not sold either way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes indeed, I copied as made sense to me

an annotation the job is probably enough

What do you mean? Like remove commenting and echo the error?

Copy link
Contributor

@JackPGreen JackPGreen Nov 11, 2024

Choose a reason for hiding this comment

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

an annotation the job is probably enough
What do you mean? Like remove commenting and echo the error?

Just echoing the error in a format like this hazelcast/hazelcast-docker#821

@JackPGreen
Copy link
Contributor

Typically (at least on open-source non-hazelcast repos) this is done via GitHub Repo config, but some Hazelcast repos use hazelcast-tpm/membership.

Fixed 967b2d882eae Pls check?

I've not done it personally so I can't say. Could setup a test repo to try it in to be sure?

@nishaatr
Copy link
Contributor Author

I've not done it personally so I can't say. Could setup a test repo to try it in to be sure?

Agreed. Let me try

@nishaatr
Copy link
Contributor Author

We also need to log that the Jenkins build should be disabled.

Mentioned in JIRA already

@nishaatr
Copy link
Contributor Author

Typically (at least on open-source non-hazelcast repos) this is done via GitHub Repo config, but some Hazelcast repos use hazelcast-tpm/membership.

Fixed 967b2d882eae Pls check?

I've not done it personally so I can't say. Could setup a test repo to try it in to be sure?

Removed external author check completely 95e8cc27c and now relying on GH setting which appears to work as intended

image

Tested this in test repo (to be deleted)

INTERNAL: https://github.com/hazelcast/test-pr-builder-nish-tmp/pull/6
EXTERNAL: https://github.com/hazelcast/test-pr-builder-nish-tmp/pull/7

image

@nishaatr
Copy link
Contributor Author

@JackPGreen
The Action build failed but Jenkins passed https://jenkins.hazelcast.com/job/hazelcast-code-samples-pr-builder/706/
Could this be down to NOT passing "-Dhazelcast.enterprise.license.key=$HZ_LICENSEKEY" to MVN or may be something else?

I checked 'HAZELCAST_ENTERPRISE_KEY_V7' is open to all repos so can't be that

@JackPGreen
Copy link
Contributor

@JackPGreen The Action build failed but Jenkins passed https://jenkins.hazelcast.com/job/hazelcast-code-samples-pr-builder/706/ Could this be down to NOT passing "-Dhazelcast.enterprise.license.key=$HZ_LICENSEKEY" to MVN or may be something else?

I checked 'HAZELCAST_ENTERPRISE_KEY_V7' is open to all repos so can't be that

Yes, it looks like LicenseUtil has it's own behaviour -

Copy link
Contributor

@JackPGreen JackPGreen left a comment

Choose a reason for hiding this comment

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

Approving for further investigation post merge due to pull_request_target.

@nishaatr nishaatr merged commit 9f5ca42 into hazelcast:master Nov 11, 2024
@JackPGreen JackPGreen mentioned this pull request Nov 11, 2024
nishaatr added a commit that referenced this pull request Nov 12, 2024
Follow-up from
#674
Add PR author check to block external PRs

---------

Co-authored-by: Jack Green <[email protected]>
nishaatr added a commit that referenced this pull request Nov 13, 2024
Follow-up from
#674
Checkout PR merge branch and not `master`

Discovered when trying to disable tests (issue with Azure identity
provider in GH). Disabling via Maven POM or \@ignore(...) or removing
the module altogether didn't work. Realized it was checking out `master`
nishaatr added a commit that referenced this pull request Nov 14, 2024
Disable test temporarily until
[DI-338](https://hazelcast.atlassian.net/browse/DI-338) is fixed

We have migrated Code Samples PR builder to GitHub
(#674) and
Hazelcast is activating Azure identity provider (likely as GH hosted
runners run in Azure) which results in the test hanging by continuously
attempting to connect to HZ cluster (which likely failed to start)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants