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

Fix serverless endless deletion/creation build job loop when using customRuntimeOverride #17553

Merged
merged 3 commits into from
May 23, 2023
Merged

Fix serverless endless deletion/creation build job loop when using customRuntimeOverride #17553

merged 3 commits into from
May 23, 2023

Conversation

dbadura
Copy link
Contributor

@dbadura dbadura commented May 23, 2023 •
edited

Description

Changes proposed in this pull request:

  • fix endless build job creation/deletion loop
  • detached function from system state

Related issue(s)
See: #17552

@kyma-bot kyma-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 23, 2023
@kyma-bot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@netlify
Copy link

netlify bot commented May 23, 2023 •
edited

✅ Deploy Preview for kyma-project-docs-preview ready!

Name Link
🔨 Latest commit a42949f
🔍 Latest deploy log https://app.netlify.com/sites/kyma-project-docs-preview/deploys/646c9f62cfeb950008a2edff
😎 Deploy Preview https://deploy-preview-17553--kyma-project-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@kyma-bot kyma-bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label May 23, 2023
@dbadura
Copy link
Contributor Author

dbadura commented May 23, 2023

/test all

@kyma-bot kyma-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels May 23, 2023
@dbadura
Copy link
Contributor Author

dbadura commented May 23, 2023

/test pull-function-controller-unit-test

@kyma-bot kyma-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 23, 2023
@dbadura dbadura changed the title fix endless loop Fix serverless endless deletion/creation build job loop May 23, 2023
@dbadura dbadura changed the title Fix serverless endless deletion/creation build job loop Fix serverless endless deletion/creation build job loop when using customRuntimeOverride May 23, 2023
@dbadura dbadura marked this pull request as ready for review May 23, 2023 11:12
@dbadura dbadura requested a review from a team as a code owner May 23, 2023 11:12
@kyma-bot kyma-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 23, 2023
@kyma-bot
Copy link
Contributor

@dbadura: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-function-controller-lint a42949f link false /test pull-function-controller-lint

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@kwiatekus
Copy link
Contributor

Tested on k3d:

kyma deploy -s PR-17553 --component serverless

After applying function with runtime image override the infinite rescheduling of build job doesnt occur

apiVersion: serverless.kyma-project.io/v1alpha2
kind: Function
metadata:
  labels:
    app.kubernetes.io/name: function-distracted-marcin5
  name: function-distracted-marcin5
  namespace: default
spec:
  replicas: 1
  resourceConfiguration:
    function:
      resources:
        limits:
          cpu: 100m
          memory: 128Mi
        requests:
          cpu: 50m
          memory: 64Mi
  runtime: nodejs18
  runtimeImageOverride: kwiatekus/nodejs18:latest
  source:
    inline:
      dependencies: |-
        {
          "name": "function-distracted-marcin5",
          "version": "0.0.1",
          "dependencies": {}
        }
      source: |-
        module.exports = {
            main: function (event, context) {
                return 'Hello Serverless'
            }
        }

@pPrecel pPrecel self-assigned this May 23, 2023
kwiatekus
kwiatekus reviewed May 23, 2023
View reviewed changes
components/function-controller/internal/controllers/serverless/function_reconcile_test.go
@@ -1305,7 +1305,62 @@ func TestFunctionReconciler_Reconcile(t *testing.T) {

g.Expect(deployment.Spec.Template.Annotations).To(gomega.HaveKeyWithValue(restartedAtAnnotationKey, restartedAtAnnotationValue))
})

t.Run("should reconcile function with RuntimeImageOverride", func(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

👏

pPrecel
pPrecel reviewed May 23, 2023
View reviewed changes
Copy link
Contributor

@pPrecel pPrecel left a comment

Choose a reason for hiding this comment

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

The code looks ok, but I'm still waiting for resolving comment from @kwiatekus

@dbadura
Copy link
Contributor Author

dbadura commented May 23, 2023

@pPrecel which comments?

@kyma-bot kyma-bot added the lgtm Looks good to me! label May 23, 2023
@kyma-bot kyma-bot merged commit 26682c3 into kyma-project:main May 23, 2023
19 of 20 checks passed
@dbadura dbadura deleted the fix-endless-build-with-runtime-override branch May 23, 2023 12:08
dbadura added a commit to dbadura/kyma that referenced this pull request May 23, 2023
@dbadura
Fix serverless endless deletion/creation build job loop when using cu…
5a6ef4c 
…stomRuntimeOverride (kyma-project#17553)

* fix endless loop

* bump images

* add unit tests
@dbadura dbadura mentioned this pull request May 23, 2023
Merged
@pPrecel
Copy link
Contributor

pPrecel commented May 23, 2023

removed one :)

@kwiatekus kwiatekus added area/serverless Issues or PRs related to serverless kind/bug Categorizes issue or PR as related to a bug. labels May 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kwiatekus kwiatekus kwiatekus left review comments

@pPrecel pPrecel pPrecel approved these changes

Assignees

@pPrecel pPrecel

Labels
area/serverless Issues or PRs related to serverless kind/bug Categorizes issue or PR as related to a bug. lgtm Looks good to me! size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@dbadura @kyma-bot @kwiatekus @pPrecel