ServiceNow / sncicd-apply-changes

MIT License
12 stars 6 forks source link

Apply Changes from Source Control is not working as expected #2

Closed chiarng closed 2 years ago

chiarng commented 3 years ago

Copying from @ramyaramdasan in https://github.com/ServiceNow/sncicd_githubworkflow/issues/4.

Apply Changes from Source Control is not working as expected. On Flow Designer and Azure Task I have used ‘Apply Changes from Source Control’ and it updated the ServiceNow instance configured as Target Instance with code from the branch assigned to the branch input. For example, both these actions had TEST as Target Instance and DEV as the branch name. However, the equivalent GitHub Action doesn’t seem to operate the same way.

Based on sample workflow provided in the repo, I configured SNOW_SOURCE_INSTANCE with the TEST instance domain name and it didn’t get updated with code from source control. When we are applying changed from Source Control to a ServiceNow Instance, shouldn't the latter be called SNOW_TARGET_INSTANCE? Also, I didn’t see an input option for source control branch.

chiarng commented 3 years ago

@ramyaramdasan Quick update for you. So yes, we confirmed that the default behavior of the "branch_name" optional parameter for the API, if provided, is to switch the branch and apply the changes from Git remote. One of our engineers looked into code and noted that the original developer was actually trying to be more user-friendly and pass in the branch name from whichever branch triggered the build. So for example, if you have a feature branch with a pull request, that feature branch would be the value passed in. If you're triggering a CD build via push to master, then the master branch would be the value passed in. In this way, the branch_name value was hidden from the end user, because usually you want to run your build to test the branch you are merging, or deploying from. There is, however, a bug. The branch_name value is being passed in as the body of the request, instead of as a query parameter as designed in the API. https://github.com/ServiceNow/sncicd-apply-changes/blob/master/src/App.ts#L107 That means it's not actually working, so the API will assume there is no branch_name value passed in, and Apply Remote Changes for whichever branch is currently active on that instance.

Our SN team needs to loop together on how we'd approach solving this.

chiarng commented 3 years ago

verified with my test pipeline that directly supplying parameter with branch name changed the branch on instance.

  - name: ServiceNow CI/CD Apply Changes
    uses: ServiceNow/sncicd-apply-changes@1.2.1
    env:
      snowUsername: ${{ secrets.SN_USERNAME }}
      snowPassword: ${{ secrets.SN_PASSWORD }}
      snowSourceInstance: ${{ secrets.SN_DEV_INSTANCE }}
      appSysID: ${{ secrets.SN_APP_SYSID }}
      branch: 'master'

next to figure out how to have dynamic branch name passed in

ramyaramdasan commented 3 years ago

@chiarnglin,

I use the below line of code to extract branch name(the one into which the push happened) and pass it as an input - ${{ steps.extract_branch.outputs.branch }}to an action in the flow. I use it in our hotfix deployment workflow to allow developers to create their own branch. I need to extract the branch names to use it in subsequent actions for merging and deleting the branch. steps:

Thanks Ramya

ramyaramdasan commented 3 years ago

verified with my test pipeline that directly supplying parameter with branch name changed the branch on instance.

  - name: ServiceNow CI/CD Apply Changes
    uses: ServiceNow/sncicd-apply-changes@1.2.1
    env:
      snowUsername: ${{ secrets.SN_USERNAME }}
      snowPassword: ${{ secrets.SN_PASSWORD }}
      snowSourceInstance: ${{ secrets.SN_DEV_INSTANCE }}
      appSysID: ${{ secrets.SN_APP_SYSID }}
      branch: 'master'

next to figure out how to have dynamic branch name passed in

@chiarnglin , I verified this too. It does solve a major concern of wrong version of application getting published from the wrong branch. The branch switching done from the pipeline seems to handle everything internally.

I updated my pipeline to use 'dev' as branch for sncicd-apply-changes and triggered the pipeline by committing work to 'dev' branch from our DEV instance. I had my TEST instance pointing to 'main' branch. It didn't switch the branch on the instance, I also checked the sys_repo_config to confirm the current branch which continued to be 'main'. While we have the ATF run on the Client Runner open on TEST instance, looks like it would still refer to the 'main' branch and not test the features that may be added to the dev branch.

Its not a showstopper, but I just thought of sharing the observations.

phifogg commented 3 years ago

@ramyaramdasan I tried in my pipeline your scenario with a little different flow. On a PR from dev branch to main I would send an apply-changes trigger to the testing instance but switching it to the dev branch. This way I get the latest version for running ATF. Once complete (failure or not) I would re-apply the main branch to reset back to what it should be.

If the test completed successfully the PR can be merged. Another apply-changes to the test instance is than required to get to the latest stage of the main branch.

I was checking if there is an option that I could update the instance to the content of the merge request... this seems not to be possible - at least to my knowledge.

ramyaramdasan commented 3 years ago

@phifogg,

What I observed is that, the Actions in pipeline is taking care of picking the correct branch for applying changes and also publish the application from that branch. But for ATF to run the new code using the Scheduled Client Runner that I opened for TEST instance, which was pointing main, it didn't switch to dev. So, in a case where we also pushed a new ATF Test corresponding to the feature that was developed under dev branch, are you saying that the instance will be having it, though we don't see a branch switch happening in the instance.

The DPP Instances have been under maintenance past many days, so I didn't really get a chance to test it. I will do so when I get my access back.

Also, when you said - "Once complete (failure or not) I would re-apply the main branch to reset back to what it should be.", are you manually doing it or is there a way to do it from pipeline?

Thanks Ramya

phifogg commented 3 years ago

@ramyaramdasan Are your ATF tests part of the same scoped application or separate? In my use case I kept the tests with the application. This way the new 'dev' tests will also be pulled from source control before executing. If your tests are in a separate scope you will need also to switch branches with another apply-from-sources action on that scope. I would though recommend to keep tests and application together.

For the re-applying main branch: I had this automated in the pipeline. Just another apply-from-sources with the main branch as target.

ramyaramdasan commented 3 years ago

@phifogg

Yes, they are in the same scope and do get deployed along with rest of the code. Probably the only point unclear to me is -

Is apply-from-sources different from sncicd-apply-changes? If they are same, once it is executed, do you see the Studio on that instance pointing to the branch for which you applied sncicd-apply-changes? We don't see that happening in our instances.

phifogg commented 3 years ago

@ramyaramdasan Yes, these actions are the same. Apply From Source is the name inside ServiceNow, sncicd-apply-changes is the name of the action on github. Indeed, in my instance I did see the branch change in Studio. We can connect on a call if you want to see yours.

ramyaramdasan commented 3 years ago

@phifogg Oh that will be great. But unfortunately I don't have any instances to walk you through the issue. Once I have them, we can surely connect.

chiarng commented 3 years ago

Sorry about the slow instance upgrades (there's some bug with the data center automation), and thank you Daniel for helping Ramya check in on this!

ramyaramdasan commented 3 years ago

hi @phifogg,

The branch name when extracted dynamically and added as an input to the Action, the step fails

steps:

extract_branch d

phifogg commented 3 years ago

HI @ramyaramdasan I have checked your setup in an example app on my side. You can see it here: https://github.com/phifogg/x_snc_jap There has to be something wrong with your branch name. It does work just fine for me, a push to my 'instances/dev' branch will trigger the pipeline and push it to my test environment (by changing its branch) and reverting that as well.

I extract the branch name using this command:

Your version looks slightly different, if I use it the resulting branch name is just 'dev' - missing the 'instances/' part.

ramyaramdasan commented 3 years ago

Hi @phifogg

Thanks for quickly checking. I did try with a few other branches with names - bug, hotfix, etc. But still it didn't work. For another workflow where I hard-coded the branch name 'dev' as input to this action, it indeed works. It switches branch automatically on the instance where its applying the changes.

I will try the command you provided above and check if that works. I will let you know the outcome.

Thanks ramya branch_extract

ramyaramdasan commented 3 years ago

Hi @phifogg,

I took a closer look at your repo (https://github.com/phifogg/x_snc_jap) and workflow executions. You were using phifogg/sncicd-apply-changes@1.2.0, while I was trying ServiceNow/sncicd-apply-changes@1.2.1. I checked the releases and there was no 1.2.0 available. The branch extraction worked fine on 1.0.0 version, so something may have changed in 1.2.1 causing the step to fail.

chiarng commented 3 years ago

@ramyaramdasan I just shipped v2.0.0 for all the GitHub Actions, do you want to check to see if this is still a problem? (Or maybe if this is a really long and old Issue, we can create new ones if there are new problems!)

ramyaramdasan commented 3 years ago

@chiarnglin,

Thanks for this note. I will definitely try the new versions on the workflows tied to Applications on the DPP Instances.

2.0.0 seems to be associated with Quebec/ Rome release. Are they Paris compatible? I wonder if I should update all the workflows currently in use on our live instances also with 2.0.0. Please suggest.

Thanks Ramya

chiarng commented 3 years ago

The APIs are backwards compatible, so the GitHub Actions should be as well... but if that's not the case and you find a bug definitely let us know!!

ramyaramdasan commented 3 years ago

Cool! I shall keep you posted if any issues. Thanks much!

phifogg commented 2 years ago

Closing issue as no more feedback - assuming it works. If not please raise another issue. Thanks.