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.

Sign up for GitHub

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

Jump to bottom

KONFLUX-5971&STONEINTG-996: set intg test status in git according to build PLR and snapshot creation status #962

Closed
wants to merge 2 commits into from
Closed

KONFLUX-5971&STONEINTG-996: set intg test status in git according to build PLR and snapshot creation status #962

wants to merge 2 commits into from

Conversation

hongweiliu17
Copy link
Contributor

  • set integration tests status to pending when pr build plr is triggered or
    retriggered
  • set integration test status to failed/cancelled when pr build plr fails
  • set integration test status to failed/cancelled with failure reason when pr snapshot is not
    created to shown to users on git provider

Signed-off-by: Hongwei Liu[email protected]

Maintainers will complete the following section

hongweiliu17
hongweiliu17 commented Dec 12, 2024
View reviewed changes
status/reporter_gitlab.go Outdated
if err != nil {
r.logger.Error(err, "failed to get PAC token from snapshot",
"snapshot.NameSpace", snapshot.Namespace, "snapshot.Name", snapshot.Name)
r.logger.Error(err, "failed to get PAC token from object",
Copy link
Contributor Author

@hongweiliu17 hongweiliu17 Dec 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@dirgim Sorry for the later response. I meant to get the kind of object in this type of log or error message. But the Object in "k8s.io/apimachinery/pkg/apis/meta/v1" doesn't support GetKind() function. Thanks!

Copy link
Contributor Author

@hongweiliu17 hongweiliu17 Dec 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I did some search and test and foundd the TypeMeta is lost when transforming data, and I plan to define a function to get object type as below. Thank you.

func GetObjectKind(obj metav1.Object) string {
	if _, ok := obj.(*applicationapiv1alpha1.Snapshot); ok {
		return "Snapshot"
	}
	if _, ok := obj.(*tektonv1.PipelineRun); ok {
		return "PipelineRun"
	}
	return ""
}

@codecov-commenter
Copy link

codecov-commenter commented Dec 12, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 71.31148% with 105 lines in your changes missing coverage. Please review.

Please upload report for BASE (main@8ac27b0). Learn more about missing BASE report.

Files with missing lines Patch % Lines
.../controller/buildpipeline/buildpipeline_adapter.go 63.35% 36 Missing and 12 partials ⚠️
status/reporter_github.go 67.50% 26 Missing ⚠️
status/reporter_gitlab.go 68.08% 13 Missing and 2 partials ⚠️
status/format.go 78.57% 6 Missing ⚠️
gitops/snapshot.go 91.83% 3 Missing and 1 partial ⚠️
status/reporter.go 62.50% 2 Missing and 1 partial ⚠️
status/status.go 89.47% 1 Missing and 1 partial ⚠️
...ntroller/buildpipeline/buildpipeline_controller.go 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@           Coverage Diff           @@
##             main     #962   +/-   ##
=======================================
  Coverage        ?   64.23%           
=======================================
  Files           ?       49           
  Lines           ?     6316           
  Branches        ?        0           
=======================================
  Hits            ?     4057           
  Misses          ?     1900           
  Partials        ?      359

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hongweiliu17 hongweiliu17 marked this pull request as draft December 13, 2024 01:14
@hongweiliu17 hongweiliu17 force-pushed the KFLUXBUGS-1761 branch 6 times, most recently from c9cde5a to 94b98c8 Compare December 17, 2024 11:00
dirgim
dirgim reviewed Dec 17, 2024
View reviewed changes
gitops/snapshot.go
scenarioContext := scenarioContext //G601
if IsContextValidForSnapshot(scenarioContext.Name, snapshot) {
return true
if snapshot, ok := object.(*applicationapiv1alpha1.Snapshot); ok {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we do something like this (untested):

                for _, scenarioContext := range scenario.Spec.Contexts {
			scenarioContext := scenarioContext //G601
                        if snapshot, ok := object.(*applicationapiv1alpha1.Snapshot); ok {
                            return IsContextValidForSnapshot(scenarioContext.Name, snapshot)
                        }
                        if plr, ok := object.(*tektonv1.PipelineRun); ok {
			    return IsContextValidForBuildPLR(scenarioContext.Name, plr)
			}
		}

Copy link
Contributor Author

@hongweiliu17 hongweiliu17 Dec 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Edit: I realize it is in a loop and return true only when IsContextValidForSnapshot/BuildPLR() is true, otherwise we need to check next context.

@hongweiliu17 hongweiliu17 force-pushed the KFLUXBUGS-1761 branch 2 times, most recently from 8e9709c to 3c89863 Compare December 24, 2024 07:37
@hongweiliu17 hongweiliu17 marked this pull request as ready for review December 24, 2024 08:03
@hongweiliu17
Copy link
Contributor Author

/retest

1 similar comment
@hongweiliu17
Copy link
Contributor Author

/retest

@hongweiliu17
feat(KONFLUX-5971): set intg test status in git according to PR bld PLR
c75fedd 
* set integration tests status to pending when build plr is triggered or
  retriggered
* set integration test status to failed/cancelled when build plr fails
* set integration test status to failed/cancelled with failure reason
  when snapshot is not created to show to users on git provider

Signed-off-by: Hongwei Liu<[email protected]>
@hongweiliu17
Copy link
Contributor Author

Checkrun example for the PR when snapshot is not created due to failing build PLR: https://github.com/hongweiliu17/devfile-sample-go-basic-1772/pull/2/checks

@hongweiliu17
Copy link
Contributor Author

As Kruno suggested in slack thread, using a fake snapshot to report integration test status to PR/MR. So I will create a new PR for this new solution.

@hongweiliu17 hongweiliu17 marked this pull request as draft January 8, 2025 10:48
@hongweiliu17
Copy link
Contributor Author

New PR #981 is created, then close this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dirgim dirgim dirgim left review comments

@jsztuka jsztuka Awaiting requested review from jsztuka jsztuka is a code owner

@Josh-Everett Josh-Everett Awaiting requested review from Josh-Everett Josh-Everett is a code owner

@kasemAlem kasemAlem Awaiting requested review from kasemAlem kasemAlem is a code owner

@sonam1412 sonam1412 Awaiting requested review from sonam1412 sonam1412 is a code owner

@dheerajodha dheerajodha Awaiting requested review from dheerajodha dheerajodha is a code owner

@14rcole 14rcole Awaiting requested review from 14rcole 14rcole is a code owner

@jencull jencull Awaiting requested review from jencull jencull is a code owner

@chipspeak chipspeak Awaiting requested review from chipspeak

Assignees
No one assigned
Labels
do-not-merge/work-in-progress size: XXL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hongweiliu17 @codecov-commenter @dirgim